Replace __eq__ by _richcmp_ for manifolds

[mjungmath]

Within the coercion frameworks, two objects are usually compared by using _richcmp_. Using this, both objects are coerced into an element of a common parent before the comparison is performed. The current implementation uses __eq__ instead and involves no prior coercion.

In this ticket, the method __eq__ is replaced by _richcmp_ as intended by the coercion framework, and __ne__ is removed. This includes the following files:

• differentiable/tensorfield.py
• section.py
• chart_func.py
• scalarfield.py
• continuous_map.py

In result, no manual coercions prior to equality checks are necessary and new coercions don't have to be added separately in the future.

To keep some equality checks function, I had to add the coercion from chart functions to scalar fields (see #30112). Furthermore, to unify the behavior for chart functions and to add this behavior into the documentation, I would like to add the dependence of #30267, too.

Depends on #30112
Depends on #30267

CC: @egourgoulhon @tscrim @mkoeppe

Component: manifolds

Author: Michael Jung

Branch/Commit: u/gh-mjungmath/test_comparison_tensorfield @ 2c06105

Reviewer: Eric Gourgoulhon, Travis Scrimshaw

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

[mjungmath]

Dependencies: #30112

[mjungmath]

Branch: u/gh-mjungmath/test_comparison_tensorfield

[mjungmath]

New commits:

3eba4e1	Trac #30108: != bug fixed
aa605ab	Trac #30108: eq check between scalar fields and mixed forms
b7d9f47	Trac #30108: wrong import
d798b17	__eq__ replaced by _richcmp_
0b93b22	Trac #30108: unused import richcmp removed
2c4e13c	Merge branch 'mixed_form_inequality_bug' into test_comparison_tensorfield
8b98301	Trac #30112: coerce map from chart function ring added
6c28c72	Merge branch 't/30112/coercion_from_chartfunction_to_scalarfield' into test_comparison_tensorfield

[mjungmath]

Commit: 6c28c72

[mjungmath]

Changed dependencies from #30112 to #30112, #30108

[sagetrac-git]

Changed commit from 6c28c72 to 979c815

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

979c815

Trac #30116: `__eq__` replaced by _richcmp_

[mjungmath]

Changed dependencies from #30112, #30108 to #30112

[sagetrac-git]

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

5fcb2e7

Trac #30116: input info op added

[sagetrac-git]

Changed commit from 979c815 to 5fcb2e7

[sagetrac-git]

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

0089c6e	Trac #30112: coercion for differentiable scalar fields
00e647d	Trac #30116: Merge branch 't/30112/coercion_from_chartfunction_to_scalarfield' into replace_eq_by_richcmp_tensorfield

[sagetrac-git]

Changed commit from 5fcb2e7 to 00e647d

[sagetrac-git]

Changed commit from 00e647d to 86a5438

[sagetrac-git]

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

385c4f9	Trac #30112: doctest added
86a5438	Trac #30116: Merge branch 't/30112/coercion_from_chartfunction_to_scalarfield' into replace_eq_by_richcmp_tensorfield

[sagetrac-git]

Changed commit from 86a5438 to 03019c1

[sagetrac-git]

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

03019c1

Trac #30116: unused import removed + replacement for sections

[mjungmath]

comment:14

I am sorry. I am too impatient. Anyway, I get the following error during the doctest:

File "src/sage/manifolds/section.py", line 2401, in sage.manifolds.section.TrivialSection.restrict
Failed example:
    s_D[[1]] == s[[1]]
Expected:
    False
Got:
    True

Here is the complete related doctest:

sage: M = Manifold(2, 'R^2')
sage: c_cart.<x,y> = M.chart() # Cartesian coordinates on R^2
sage: E = M.vector_bundle(2, 'E')
sage: e = E.local_frame('e') # makes E trivial
sage: s = E.section(x+y, -1+x^2, name='s')
sage: D = M.open_subset('D') # the unit open disc
sage: e_D = e.restrict(D)
sage: c_cart_D = c_cart.restrict(D, x^2+y^2<1)
sage: s_D = s.restrict(D) ; s_D
Section s on the Open subset D of the 2-dimensional differentiable
 manifold R^2 with values in the real vector bundle E of rank 2

...

sage: s_D[[1]] == s[[1]]
False

With respect to the coercion model, the output is perfectly fine: the section s can be restricted to D. Thus there exists a coercion from sections on M to sections on D. The equality check then takes places on the sections on D.

Should I simply rewrite this doctest, or what is the best procedure here?

For consistency, the same should then hold for chart functions, too. Meaning the following command right above should then return True as well:

sage: s_D[1] == s[1]

[mjungmath]

comment:55

There was a merge conflict with the new beta. The cause was #30108. Can I switch back to positive review when the bot turns green?

[tscrim]

comment:56

Morally green patchbot.

comment:52 response: I have no objections.

[mjungmath]

comment:57

Matthias mentioned a very important point. The equality via restriction doesn't satisfy the transitivity condition. That is because the scalar fields are not necessarily analytic. That's really bad.

Is there a compromise, namely keeping _richcmp_ and the coercion but separate this particular case? I don't know of any.

[mkoeppe]

comment:58

As I mentioned in #30266, I don't think you should feel obligated to use the coercion framework for comparisons just because you use it for arithmetic.

I would suggest to compile a set of examples that really illustrate what elements you want to compare equal, to make sure that this is an equivalence relation, and then to figure out if coercion is the right tool to implement this.

Perhaps the question should be asked in a broader context of coercion within the category of partial maps.

[tscrim]

comment:59

Replying to @mjungmath:

Matthias mentioned a very important point. The equality via restriction doesn't satisfy the transitivity condition. That is because the scalar fields are not necessarily analytic. That's really bad.

That sounds like an issue with the coercions. However, if you want them to also compare as equals without the coercion, then you will need to simply implement an __eq__ that separately checks this as a fallback provided the first attempt via the coercion framework doesn't work.

[mjungmath]

comment:61

Okay, just to summarize for me: what is the punchline of our discussion and where should this ticket go?

[tscrim]

comment:62

I would say Matthias's comment:58 is what should be done: provide some examples of things you want and do not want to inform you of the policy you want to set.

[mkoeppe]

comment:63

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

[mjungmath]

comment:64

Possibly related: #31703

[mkoeppe]

comment:65

Setting a new milestone for this ticket based on a cursory review.

[mkoeppe]

comment:66

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.