#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 | c38ae80cbd8032cf7041259284b6f646265d1e42
Branch: | Stopgaps:
public/manifolds/top_manif_basics |
Dependencies: #18175 |
-------------------------------------+-------------------------------------
Comment (by tscrim):
Sorry for the delay in getting to this.
Replying to [comment:89 egourgoulhon]:
> - revert the attribute `_open_covers` to a list of lists (instead of a
set of frozensets), mostly to
> ensure the reproductibility of computations from one machine to
another one, i.e. this garantees
> that `for ... in` loops are always performed in the same way (maybe
this is something to
> be discussed further...). Moreover, the mathematical definition of an
> open cover refers to an indexed family, which corresponds more to a
list than to a frozenset.
The `set` of `frozenset`s will have a faster lookup as manifolds get
larger. I think this is still valid mathematically because the indexed
family does not have to be ordered as far as I know. So it would be more
like a `dict`, but I don't think we care about the indexing; if we did,
then we should go to a `list`. For the doctests, we can just do a `sorted`
or we have an internal method which returns it in a canonical way (i.e.,
ordered via its string representation).
> - removed the method `is_open()` from `AbstractSet` (where it was always
returning `True`!) and
> implemented it in `TopologicalManifold`.
Perhaps the default value for `is_open` in `AbstractSet` should return an
`Unknown`, be an `@abstract_method`, or raise a `NotImplementedError`? I'm
in favor of the first one.
> - removed the (unused) argument `category` from
`ManifoldSubset.__init__`
This was previously there as I was wanting to call the `__init__`, but
since I copied the constructor, this has become unnecessary. Although now
I'm thinking we should have an `_init_subset` method to remove the
duplicated code and have that be called by both `ManifoldSubset.__init__`
and `OpenTopologicalSubmanifold.__init__`.
> - added an example to illustrate the class `OpenTopologicalSubmanifold`,
as well as enough doctests
> to get a full coverage
Thank you.
> To answer to some of your questions in the code:
> - I don't think that `ManifoldPoint` must inherit from
`AbstractNamedObject` since the name is
> not essential in the definition of a point (it can have no name),
contrary to that of a subset.
> Moreover, it would slower the creation of points, which can be a
problem (no such problem for
> parent objects, which are not expected to be massively created)
Fair enough. Although if that extra little function call is going to
matter, then I think we should cythonize `ManifoldPoint`.
> - Yes, I think we need `list_of_subsets` to have something duplicable
from one machine to another one
> (cf. remark above about `_open_covers`).
Same comments about `_open_covers` as well.
> The test suite of `ManifoldSubset` and `OpenTopologicalSubmanifold`
fails because of the lack of a method `lift`. What this method shall be? I
guess we have to wait for the morphism ticket (#18725) to implement it.
`lift` can just be a method that creates the corresponding point in the
ambient manifold, and similarly for `retract`. Once we have #18725, we can
convert it to a `@lazy_attribute` which is set to the morphism (note that
this does not change the API).
I don't reall agree with comment:92. I feel that this only removes a few
docstrings, adds some internal complexity, and increases memory usage
(although since we aren't really using this for `TopologicalPoint`, the
memory usage is less of a concern). In principle, it is a mixin class, but
it just acts like an ABC. So it is perfectly valid to override its
`_repr_`.
I'm glad to hear the mixin class idea is working.
--
Ticket URL: <http://trac.sagemath.org/ticket/18529#comment:95>
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.
