#18529: Topological manifolds: basics
-------------------------------------+-------------------------------------
Reporter: egourgoulhon | Owner: egourgoulhon
Type: enhancement | Status: needs_info
Priority: major | Milestone: sage-7.0
Component: geometry | Resolution:
Keywords: topological | Merged in:
manifolds | Reviewers: Travis Scrimshaw
Authors: Eric Gourgoulhon, | Work issues:
Travis Scrimshaw | Commit:
Report Upstream: N/A | 3cd03a48d847e12745ed8c25b23f19db141c179a
Branch: | Stopgaps:
public/manifolds/top_manif_basics |
Dependencies: #18175 |
-------------------------------------+-------------------------------------
Comment (by egourgoulhon):
Replying to [comment:108 tscrim]:
>
> You had many `if self._manifold is self:` statements, which breaks the
localization principle;
Well not so many: grep returns only 5 occurences in the whole
!SageManifolds:
- in `TopologicalManifold._repr_` for returning "Manifold..." instead of
"Open subset of manifold..."
- in `DifferentiableManifold._repr_` for the same reason
- in `ManifoldSubset.superset`, because a superset can only `self` in case
of the whole manifold
- in `ManifoldSubset.intersection`, because the intersection with the
whole manifold is trivial
- in `ManifoldSubset.union`, because the union with the whole manifold is
trivial
>
> Additionally, subsets of a manifold are precisely that, subsets. So we
should encapsulate that in separate classes. Mathematically we identity an
open subset as a manifold, but by doing so, we loose the information that
it is really a subset of another manifold. As such, I feel that having
separate classes actually better reflects the mathematics.
Actually, at the moment, we are dealing with the open subsets in exactly
the same way as we are doing with the whole manifold, i.e. we set up
charts on them, define tensor fields, etc. This can be seen by noticing
that in Hierarchy-1, the class `OpenTopSubmanifold` has very few methods
in addition to those inherited from `TopologicalManifold`. Similarly, none
of the classes `OpenDiffSubmanifold` and `DifferentiableManifold`
introduces any attribute or method by itself, in addition to those
inherited from `DifferentiableMixin` (except for their `__init__`).
Maybe, in a future development, one could have operations that make sense
only for open subsets or only for the whole manifold. Then having two
separate classes is clearly the way to go. However, at the moment I don't
see any such operation, apart from those listed above: the printout (via
`_repr_`) and the union/intersection with other subsets.
>
> In an ideal world, I would like to have a hierarchy similar to
Hierachy-2, but with an additional subclass corresponding to each type of
manifold that is obtained by adding a subset mixin class. Yet, that
doesn't seem possible with the current core design.
>
> While the Hierarchy-1 seems more complicated, it actually will be easier
to maintain and debug as code is local to the class (i.e., if you are a
subclass of `A`, then you act in the same way and like `A` irregardless of
your input).
>
> This assessment is based on my experiences and what I learned as good
OOP principles. However, I have not looked at !SageManifolds beyond this
ticket at present, whereas you have worked heavily on it. So perhaps we
can get some more data. How many times before the refactoring did you have
to do a `self._manifold is self` test?
Five times, as mentioned above.
> Also what is your thoughts on maintenance and extendability?
It seems to me that, in terms of maintenance, Hierarchy-2 is better, being
simpler; it is also certainly better for somebody new entering into the
code.
On the contrary, if one regards extendability, Hierarchy-1 offers the
possibility to have different operations for manifolds and subsets, as
mentioned above. This is a good point in favor of it. Even if at the
moment I don't have in mind any such operation (apart from those listed
above), I would slightly tend to maintain Hierarchy-1 because of this.
>
> Something else might be to get a 3rd party's opinion (Nicolas and/or
Darij comes to my mind).
Yes extra points of view would be helpful! In addition to Nicolas and
Darij, I don't know if other people associated to this ticket (Vincent,
John, Nils, Jeoren,...) are reading this and have an opinion...
> I could also sit down and plan out how I would do everything from the
ground up to see if I can devise a better overall strategy if you think
there might be some benefit to that at this stage.
Thanks, but maybe this is not necessary at this stage, especially if we
conclude that we maintain Hierarchy-1. Since all this has little impact on
the user interface, maybe we could rediscuss it after having a whole view
of all the manifold tickets.
--
Ticket URL: <http://trac.sagemath.org/ticket/18529#comment:109>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.