Topological manifolds: basics

[egourgoulhon]

This is the implementation of topological manifolds over a topological field K resulting from the SageManifolds project. See the meta-ticket #18528 for an overview. By topological manifold over a topological field K it is meant a second countable Hausdorff space M such that every point in M has a neighborhood homeomorphic to Kⁿ, with the same non-negative integer n for all points.

This tickets implements the following Python classes:

• ManifoldSubset: generic subset of a topological manifold (the open subsets being implemented by the subsclass TopologicalManifold)
  • TopologicalManifold: topological manifold over a topological field K
• ManifoldPoint: point in a topological manifold
• Chart: chart of a topological manifold
  • RealChart: chart of a topological manifold over the real field
• CoordChange: transition map between two charts of a topological manifold

as well as the singleton classesTopologicalStructure and RealTopologicalStructure.

TopologicalManifold is intended to serve as a base class for specific manifolds, like smooth manifolds (K=R) and complex manifolds (K=C). The follow-up ticket, implementing continuous functions to the base field, is #18640.

Documentation: The reference manual is produced by sage -docbuild reference/manifolds html It can also be accessed online at http://sagemanifolds.obspm.fr/doc/18529/reference/manifolds/ More documentation (e.g. example worksheets) can be found here.

Depends on #18175

CC: @man74cio

Component: geometry

Keywords: topological manifolds

Author: Eric Gourgoulhon, Travis Scrimshaw

Branch/Commit: 00d265c

Reviewer: Travis Scrimshaw, Eric Gourgoulhon

Issue created by migration from https://trac.sagemath.org/ticket/18529

[egourgoulhon]

Description changed:

--- 
+++ 
@@ -1 +1,11 @@
-This is the implementation of topological manifolds over some field K resulting from the [SageManifolds project](http://sagemanifolds.obspm.fr/). See the meta-ticket #18528 for an overview.
+This is the implementation of topological manifolds over a field K resulting from the [SageManifolds project](http://sagemanifolds.obspm.fr/). See the meta-ticket #18528 for an overview.
+
+This tickets implements the following Python classes:
+
+- `TopManifold`: topological manifold over a field K
+- `TopManifoldPoint`: point in a topological manifold
+- `TopManifoldSubset`: generic subset of a topological manifold
+  - `TopManifoldOpenSubset`: open subset of a topological manifold
+- `Chart`: chart of a topological manifold
+  - `RealChart`: chart of a topological manifold over the real field
+- `CoordChange`: transition map between two charts of a topological manifold

[jhpalmieri]

comment:2

The phrase "manifold over a field K" sounds odd to me. Is it used in the literature? What if K is a finite field? It seems that if X is a finite discrete space, for every finite field F and for every non-negative integer n, then X is a manifold over F of dimension n: F and n play no role. (I'm assuming that finite fields have been given the discrete topology.)

I think you might say "topological manifold over a topological field K", since obviously the topology on K is critical. Or you could omit "over a field K", and mention in the documentation that users can specify a topological field (like \CC, rather than the default \RR) if they want.

[egourgoulhon]

Description changed:

--- 
+++ 
@@ -1,11 +1,14 @@
-This is the implementation of topological manifolds over a field K resulting from the [SageManifolds project](http://sagemanifolds.obspm.fr/). See the meta-ticket #18528 for an overview.
+This is the implementation of topological manifolds over a topological field K resulting from the [SageManifolds project](http://sagemanifolds.obspm.fr/). See the meta-ticket #18528 for an overview.
+By *topological manifold over a topological field K* it is meant a second countable Hausdorff space M such that every point in M has a neighborhood homeomorphic to K<sup>n</sup>, with the same non-negative integer n for all points. 

 This tickets implements the following Python classes:

-- `TopManifold`: topological manifold over a field K
+- `TopManifold`: topological manifold over a topological field K
 - `TopManifoldPoint`: point in a topological manifold
 - `TopManifoldSubset`: generic subset of a topological manifold
   - `TopManifoldOpenSubset`: open subset of a topological manifold
 - `Chart`: chart of a topological manifold
   - `RealChart`: chart of a topological manifold over the real field
 - `CoordChange`: transition map between two charts of a topological manifold
+
+`TopManifold` is intended to serve as a base class for specific manifolds, like smooth manifolds (K=**R**) and complex manifolds (K=**C**). 

[egourgoulhon]

comment:4

Replying to @jhpalmieri:

The phrase "manifold over a field K" sounds odd to me. Is it used in the literature?

Thanks for your comment. You are right: this is an abusive generalization of "manifold over R" and "manifold over C", which are used in the literature.

What if K is a finite field? It seems that if X is a finite discrete space, for every finite field F and for every non-negative integer n, then X is a manifold over F of dimension n: F and n play no role. (I'm assuming that finite fields have been given the discrete topology.)

I think you might say "topological manifold over a topological field K", since obviously the topology on K is critical. Or you could omit "over a field K", and mention in the documentation that users can specify a topological field (like \CC, rather than the default \RR) if they want.

Thanks for your suggestion; I've modified the ticket description accordingly. I've also added what is meant by "topological manifold over a topological field K".

PS: note that the code in the associated branch is still in a very crude draft state, but should be ready for review within a few days.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 89c063c to 5a5722b

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

5a5722b

Add doctests in classes Chart and RealChart

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

4f490af

Improve the documentation of coordinate charts

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 5a5722b to 4f490af

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 4f490af to d8df59f

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

d8df59f

Improve the documentation of TopManifold

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

fb96562

Open subsets of topological manifolds are now fully considered as topological manifolds.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from d8df59f to fb96562

[egourgoulhon]

Description changed:

--- 
+++ 
@@ -6,9 +6,8 @@
 - `TopManifold`: topological manifold over a topological field K
 - `TopManifoldPoint`: point in a topological manifold
 - `TopManifoldSubset`: generic subset of a topological manifold
-  - `TopManifoldOpenSubset`: open subset of a topological manifold
 - `Chart`: chart of a topological manifold
   - `RealChart`: chart of a topological manifold over the real field
 - `CoordChange`: transition map between two charts of a topological manifold

-`TopManifold` is intended to serve as a base class for specific manifolds, like smooth manifolds (K=**R**) and complex manifolds (K=**C**). 
+`TopManifold` is intended to serve as a base class for specific manifolds, like smooth manifolds (K=**R**) and complex manifolds (K=**C**).

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

38c3c12

Improve documentation of classes TopManifold, TopManifoldSubset and TopManifoldPoint

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from fb96562 to 38c3c12

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 38c3c12 to 7809ebf

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

7809ebf

Minor modifications in classes TopManifold and Chart.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 7809ebf to 99cc8c1

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

99cc8c1

Introduce Chart._init_coordinates() to simplify constructors of Chart and RealChart

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

26fc318

Modifications in CoordChange to allow for subclasses.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 99cc8c1 to 26fc318

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

a74c2e0

Add example of p-adic manifold

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 26fc318 to a74c2e0

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from a74c2e0 to 542b82a

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

542b82a

Suppress verbose in TestSuite().run; minor improvements in documentation

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

26c4890

Minor improvements in the doc of topological manifolds (basics part)

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 542b82a to 26c4890

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

be3ff74

Introduce open covers on top manifolds

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 26c4890 to be3ff74

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from be3ff74 to 4de19a7

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

4de19a7

Merge branch 'public/manifolds/top_manif_basics' of git://trac.sagemath.org/sage into sage 6.9

[egourgoulhon]

Description changed:

--- 
+++ 
@@ -11,3 +11,10 @@
 - `CoordChange`: transition map between two charts of a topological manifold

 `TopManifold` is intended to serve as a base class for specific manifolds, like smooth manifolds (K=**R**) and complex manifolds (K=**C**).
+
+**Documentation**:
+The reference manual is produced by
+`sage -docbuild reference/manifolds html`
+It can also be accessed online at http://sagemanifolds.obspm.fr/doc/18529/reference/manifolds/
+More documentation (e.g. example worksheets) can be found [here](http://sagemanifolds.obspm.fr/documentation.html).
+

[videlec]

comment:20

Why did you abreviate TopologicalManifold? Everywhere in Sage classes have plain names like PolynomialRing and not PolRing.

[egourgoulhon]

comment:21

Replying to @videlec:

Why did you abreviate TopologicalManifold?

No strong argument, except that this is shorter to write (well, thanks to the tab key this is a pretty weak argument) and it is a reminder of Top, which is the standard name for the category of topological manifolds. Similarly, we useDiffManifold for differentiable manifold, Diff being the standard name of the category of differentiable manifolds.

Everywhere in Sage classes have plain names like PolynomialRing and not PolRing.

This is indeed a strong point: naming conventions should be homogeneous all across Sage. So I am considering to change everywhere TopManifold to TopologicalManifold and DiffManifold to DifferentiableManifold.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 4de19a7 to 6dec6d5

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

3647305	Some cleanup and adding more category information to particular sets.
f810aa6	Reworking the category of manifolds.
375ff46	Fixing doctest failures and letting a few other rings know they are metric spaces.
8b851a0	Merge branch 'develop' into public/categories/topological_metric_spaces-18175
f8f5b93	Fixing last remaining doctests.
041a5d1	Adding p-adics to metric spaces and some cleanup.
bfa0cdf	One last doc tweak.
d13c368	Fixing doc of metric spaces.
2605c0b	Merge #18529 (Topological manifolds: basics) into #18175 (Implement categories for topological...)
6dec6d5	Implement topological manifolds (basics, #18529) on the new categories for manifolds (#18175)

[tscrim]

comment:23

I agree with Vincent here; we should be explicit in our names.

Although is there a reason why you just did not have a single Manifold function/class as a global entry point, which then could delegate out to *Manifold depending on some boolean input arguments? I feel that this would make the user interface easier and have a single collective point for general documentation. Another option I see would be to have a manifolds catalog which then would have things like Topological, Differentiable, etc.

[egourgoulhon]

comment:24

Yes, it is pretty clear that we should have the following renaming of classes:

• TopManifold --> TopologicalManifold
• DiffManifold --> DifferentiableManifold

Regarding the user interface, we could indeed have a function Manifold that looks like

Manifold(dim, symbol, type='smooth', ...) 

with 'topological', 'differentiable', 'smooth' as possible values for the parameter type (as well as 'analytic', etc. in the future). A question: should 'smooth' be the default value?

Besides, I remember someone was complaining that Manifold was already used in SnapPy, cf. this page, but is there any danger of name clash? i.e. is SnapPy used (or could be used in the future) from Sage?

[tscrim]

comment:25

Replying to @egourgoulhon:

Regarding the user interface, we could indeed have a function Manifold that looks like

Manifold(dim, symbol, type='smooth', ...) 

with 'topological', 'differentiable', 'smooth' as possible values for the parameter type (as well as 'analytic', etc. in the future). A question: should 'smooth' be the default value?

I don't know how easy it would be to make this a check on the atlas used to construct the manifold. I would make it select it automatically if possible, and then default to the most general setting of 'topological'.

Besides, I remember someone was complaining that Manifold was already used in SnapPy, cf. this page, but is there any danger of name clash? i.e. is SnapPy used (or could be used in the future) from Sage?

I would then add an additional argument of implementation, which would default to the native Sage implementation, if there does become a name clash and have the constructor delegate out to the SnapPy version when it exists in Sage (IDK if it is Sage now or not).

[egourgoulhon]

comment:26

Replying to @tscrim:

I don't know how easy it would be to make this a check on the atlas used to construct the manifold. I would make it select it automatically if possible, and then default to the most general setting of 'topological'.

Actually in the current setting, the atlas is not specified at the construction of the manifold: charts and transition maps are introduced on the fly, at any step in the user workflow. Therefore the type of manifold has to be declared explicitly by the user. It is my impression (but I may be biased) that one speaks about a "manifold" without any qualifier, one implicitely means a "real smooth manifold". Therefore I would set the default arguments of the Manifold function to field='real' and type='smooth'.

I would then add an additional argument of implementation, which would default to the native Sage implementation, if there does become a name clash and have the constructor delegate out to the SnapPy version when it exists in Sage (IDK if it is Sage now or not).

Good idea!

[tscrim]

comment:27

Some things from looking over the current structure:

• I would separate out parts of the subset class that applies to Top(ological)Manifold and TopManifoldSubset into an ABC (abstract base class) so you don't have to do things like self is manifold.

• Maybe put the subsets into the Subobjects class and maybe consider not making it a facade?

• Should manifolds and their subsets be UniqueRepresentation? I understand that the name as the only input means they are essentially treated as variables, but it means you have to do things like TopManifold._clear_cache_() (which may be needed not just necessarily in doctests if one does this before the gc comes around).

• Related to the above, we want parents to be hashable and have a good equality. I know that by making it a UniqueRepresentation, many of these issues are solved. However the manifolds are essentially mutable, but by doing a __hash__ which only depends upon the name(s) and dimension means this is okay (__eq__ defaults to check by is). This would alleviate the need for UniqueRepresentation.

• In the tests for _element_constructor_, you shouldn't call it explicitly but via X(...), which also checks that it is working correctly with the coercion framework.

• I don't quite agree with checking self._field == RR for real charts as the precision should not matter. I would check isinstance(self._field, RealField) instead. Granted, we probably should have a function that checks against all known real field implementations...

• In a similar vein, I don't like the input of 'real' to RR (and 'complex' to CC). I would make the user be explicit about what they want unless you are doing to do some special handling to make this pass special arguments to an underlying SR implementation.

[egourgoulhon]

comment:28

Replying to @tscrim:

Thanks for your comments/suggestions.

Some things from looking over the current structure:

• I would separate out parts of the subset class that applies to Top(ological)Manifold and TopManifoldSubset into an ABC (abstract base class) so you don't have to do things like self is manifold.

I am not sure an ABC would help here: this would make a clear distinction between the manifold and strict subsets of it (thus avoiding the very few tests self is self._manifold), but on the other hand, we need open strict subsets to be in the class TopologicalManifolds.

• Maybe put the subsets into the Subobjects class and maybe consider not making it a facade?

I had the impression that the facade feature is exactly what we need, since we can have the same point created from different facade parents by means of different charts: if U and V are two overelapping open subsets of manifold M, it may happen that the same point of M is declared in two different ways:

   p = U((x,y), chart=C1)
   q = V((u,v), chart=C2)

Thanks to the facade mecanism, p and q will have the same parent: M and then it is possible to check whether p == q; this will hold if the coordinates (x,y) in chart C1 correspond to coordinates (u,v) in chart C2. Is there something bad in using facade parents?

• Should manifolds and their subsets be UniqueRepresentation? I understand that the name as the only input means they are essentially treated as variables, but it means you have to do things like TopManifold._clear_cache_() (which may be needed not just necessarily in doctests if one does this before the gc comes around).

Actually, I don't see any use case (except for confusing the reader!) where one would define two different manifolds with the same name and same dimension over the same topological field. So UniqueRepresentation seems quite appropriate here. Moreover, I thought this is Sage's credo to have parents be UniqueRepresentation, cf. the phrase You are encouraged to make your parent "unique" in the tutorial How to implement new algebraic structures in Sage. Also, most parents in src/sage/geometry (in particular the hyperbolic plane) have UniqueRepresentation.

• Related to the above, we want parents to be hashable and have a good equality. I know that by making it a UniqueRepresentation, many of these issues are solved.

Indeed!

However the manifolds are essentially mutable, but by doing a __hash__ which only depends upon the name(s) and dimension means this is okay (__eq__ defaults to check by is). This would alleviate the need for UniqueRepresentation.

Why should we avoid UniqueRepresentation? It's true that manifolds are mutable as Python objects, since the main idea is to add charts (and vector frames for differentiable manifolds) "on the fly" (for, in a concrete calculation, charts are often defined from previous ones by some transition map that might depend on some computational result and therefore it is impossible to introduce all charts at the instantiation of the manifold object). But this mutability is "secondary": the primary attributes of a manifold being its dimension, its name and its base field. With respect to these, the manifold is immutable. The need for _clear_cache_() in some doctests seems a very small annoyment, with respect to the benefit of UniqueRepresentation, doesn't it ? (I am afraid I have not understood the non-doctest case involving the garbage collector that you mentioned)

• In the tests for _element_constructor_, you shouldn't call it explicitly but via X(...), which also checks that it is working correctly with the coercion framework.

OK, I will change this.

• I don't quite agree with checking self._field == RR for real charts as the precision should not matter. I would check isinstance(self._field, RealField) instead.

Yes, I agree. I was also not satisfied with this.

Granted, we probably should have a function that checks against all known real field implementations...

• In a similar vein, I don't like the input of 'real' to RR (and 'complex' to CC). I would make the user be explicit about what they want unless you are doing to do some special handling to make this pass special arguments to an underlying SR implementation.

Yes, I agree too.

[egourgoulhon]

comment:29

Replying to @egourgoulhon:

Replying to @tscrim:

• I would separate out parts of the subset class that applies to Top(ological)Manifold and TopManifoldSubset into an ABC (abstract base class) so you don't have to do things like self is manifold.

I am not sure an ABC would help here: this would make a clear distinction between the manifold and strict subsets of it (thus avoiding the very few tests self is self._manifold), but on the other hand, we need open strict subsets to be in the class TopologicalManifolds.

I gave a second thought to this: are you thinking about something like

                               The_ABC
                              /      \
  TopologicalManifoldStrictSubset  TopologicalManifold
                              \      /
                 TopologicalManifoldStrictOpenSubset

with the methods superset(), intersection() and union() being implemented in each of the classes TopologicalManifoldStrictSubset and TopologicalManifold ?

[tscrim]

comment:30

In a short word, yes, that is correct. I will respond in more detail tomorrow morning when I wake up.

[tscrim]

comment:31

Replying to @egourgoulhon:

Replying to @tscrim:

Thanks for your comments/suggestions.

Some things from looking over the current structure:

• I would separate out parts of the subset class that applies to Top(ological)Manifold and TopManifoldSubset into an ABC (abstract base class) so you don't have to do things like self is manifold.

I am not sure an ABC would help here: this would make a clear distinction between the manifold and strict subsets of it (thus avoiding the very few tests self is self._manifold), but on the other hand, we need open strict subsets to be in the class TopologicalManifolds.

The diagram you have below is what I was thinking (at least the first 2 levels). This gives better separation of concerns and better distinguishes the two classes.

• Maybe put the subsets into the Subobjects class and maybe consider not making it a facade?

I had the impression that the facade feature is exactly what we need, since we can have the same point created from different facade parents by means of different charts: if U and V are two overelapping open subsets of manifold M, it may happen that the same point of M is declared in two different ways:

   p = U((x,y), chart=C1)
   q = V((u,v), chart=C2)

Thanks to the facade mecanism, p and q will have the same parent: M and then it is possible to check whether p == q; this will hold if the coordinates (x,y) in chart C1 correspond to coordinates (u,v) in chart C2. Is there something bad in using facade parents?

It depends on what you want the points associated with. If they are to be considered proper subsets of the manifold, where you want to do operations, then the point should know that it belongs to the subset. If it is really just reflecting a particular chart, then you probably should reconsider your entire class hierarchy because I think it shouldn't quack like a manifold in that case.

It really comes down to what you're doing mostly in the code. If you'd be doing a lot of conversions between points thought of as being in the subset and as in the manifold, then a facade is probably an okay way to go. If you care about associating the point with a particular subset, then it should not be a facade.

• Should manifolds and their subsets be UniqueRepresentation? I understand that the name as the only input means they are essentially treated as variables, but it means you have to do things like TopManifold._clear_cache_() (which may be needed not just necessarily in doctests if one does this before the gc comes around).

Actually, I don't see any use case (except for confusing the reader!) where one would define two different manifolds with the same name and same dimension over the same topological field. So UniqueRepresentation seems quite appropriate here. Moreover, I thought this is Sage's credo to have parents be UniqueRepresentation, cf. the phrase You are encouraged to make your parent "unique" in the tutorial How to implement new algebraic structures in Sage. Also, most parents in src/sage/geometry (in particular the hyperbolic plane) have UniqueRepresentation.

The hyperbolic plane works well because it is uniquely defined. Also not all parents are UniqueRepresentations, the reason why we do this is because of the coercion framework and speed. Identity checks are typically much faster than equality checks.

However the manifolds are essentially mutable, but by doing a __hash__ which only depends upon the name(s) and dimension means this is okay (__eq__ defaults to check by is). This would alleviate the need for UniqueRepresentation.

Why should we avoid UniqueRepresentation? It's true that manifolds are mutable as Python objects, since the main idea is to add charts (and vector frames for differentiable manifolds) "on the fly" (for, in a concrete calculation, charts are often defined from previous ones by some transition map that might depend on some computational result and therefore it is impossible to introduce all charts at the instantiation of the manifold object). But this mutability is "secondary": the primary attributes of a manifold being its dimension, its name and its base field. With respect to these, the manifold is immutable. The need for _clear_cache_() in some doctests seems a very small annoyment, with respect to the benefit of UniqueRepresentation, doesn't it ? (I am afraid I have not understood the non-doctest case involving the garbage collector that you mentioned)

This comes down to the manifold not being well-defined by its dimension, name, and base field. It also needs to be defined by its chart. I understand that giving two distinct manifolds the same name is bad mathematically, and you are reflecting that in your code. However what if I create a manifold M, then decide I want to create a new (and different) manifold M of the same dimension over the same base field? When I try to do this naïvely, I get all of the previous chart information much to my surprise (the garbage collector this was more about temporary objects).

[egourgoulhon]

comment:32

Replying to @tscrim:

It really comes down to what you're doing mostly in the code. If you'd be doing a lot of conversions between points thought of as being in the subset and as in the manifold, then a facade is probably an okay way to go. If you care about associating the point with a particular subset, then it should not be a facade.

I think we are precisely in the first case. In particular we do not care about associating the point with a particular subset (since many overlapping subsets can be introduced that contain the point and none of them is to be privileged with respect to the point). So I would say that we should use the facade.

The hyperbolic plane works well because it is uniquely defined. Also not all parents are UniqueRepresentations, the reason why we do this is because of the coercion framework and speed. Identity checks are typically much faster than equality checks.

Thanks for these explanations.

However the manifolds are essentially mutable, but by doing a __hash__ which only depends upon the name(s) and dimension means this is okay (__eq__ defaults to check by is). This would alleviate the need for UniqueRepresentation.

Why should we avoid UniqueRepresentation? It's true that manifolds are mutable as Python objects, since the main idea is to add charts (and vector frames for differentiable manifolds) "on the fly" (for, in a concrete calculation, charts are often defined from previous ones by some transition map that might depend on some computational result and therefore it is impossible to introduce all charts at the instantiation of the manifold object). But this mutability is "secondary": the primary attributes of a manifold being its dimension, its name and its base field. With respect to these, the manifold is immutable. The need for _clear_cache_() in some doctests seems a very small annoyment, with respect to the benefit of UniqueRepresentation, doesn't it ? (I am afraid I have not understood the non-doctest case involving the garbage collector that you mentioned)

This comes down to the manifold not being well-defined by its dimension, name, and base field. It also needs to be defined by its chart.

Indeed. More precisely, a manifold is defined by its maximal atlas. But there is no way to represent the latter in Sage, nor in any computer system. Accordingly, we cannot check the equality of two manifolds. This is why equality by id seems reasonable here.

I understand that giving two distinct manifolds the same name is bad mathematically, and you are reflecting that in your code. However what if I create a manifold M, then decide I want to create a new (and different) manifold M of the same dimension over the same base field? When I try to do this naïvely, I get all of the previous chart information much to my surprise (the garbage collector this was more about temporary objects).

You are right: a user may want to restart some computation by creating a manifold with the same name as that used previously. If he is doing so in the same session, the UniqueRepresentation behavior will return the previously created object. We cannot demand that the user runs _clear_cache_() before creating the manifold...

[tscrim]

comment:33

Replying to @egourgoulhon:

Replying to @tscrim:

It really comes down to what you're doing mostly in the code. If you'd be doing a lot of conversions between points thought of as being in the subset and as in the manifold, then a facade is probably an okay way to go. If you care about associating the point with a particular subset, then it should not be a facade.

I think we are precisely in the first case. In particular we do not care about associating the point with a particular subset (since many overlapping subsets can be introduced that contain the point and none of them is to be privileged with respect to the point). So I would say that we should use the facade.

Then we go with that.

This comes down to the manifold not being well-defined by its dimension, name, and base field. It also needs to be defined by its chart.

Indeed. More precisely, a manifold is defined by its maximal atlas. But there is no way to represent the latter in Sage, nor in any computer system. Accordingly, we cannot check the equality of two manifolds. This is why equality by id seems reasonable here.

Then we are in agreement: equality by id.

I understand that giving two distinct manifolds the same name is bad mathematically, and you are reflecting that in your code. However what if I create a manifold M, then decide I want to create a new (and different) manifold M of the same dimension over the same base field? When I try to do this naïvely, I get all of the previous chart information much to my surprise (the garbage collector this was more about temporary objects).

You are right: a user may want to restart some computation by creating a manifold with the same name as that used previously. If he is doing so in the same session, the UniqueRepresentation behavior will return the previously created object. We cannot demand that the user runs _clear_cache_() before creating the manifold...

As I outlined above, it should be easy enough to remove the UniqueRepresentation behavior and still retain the desirable behaviors such as hashability.

[egourgoulhon]

comment:34

Replying to @tscrim:

As I outlined above, it should be easy enough to remove the UniqueRepresentation behavior and still retain the desirable behaviors such as hashability.

I am on it and should push a new commit soon.

[tscrim]

comment:35

Replying to @egourgoulhon:

Replying to @tscrim:

As I outlined above, it should be easy enough to remove the UniqueRepresentation behavior and still retain the desirable behaviors such as hashability.

I am on it and should push a new commit soon.

Thanks. Also thank you for all your work on this.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 6dec6d5 to f342e03