Trac #32929: Bad determination of the coordinate range when restricti…

…ng charts to subdomains

Since Sage 9.4, we have
{{{
sage: M = Manifold(2, 'M')
sage: X.<x,y> = M.chart(r"x:(0,+oo) y:(0,2):periodic")
sage: X.coord_range()
x: (0, +oo); y: [0, 2] (periodic)
sage: U = M.open_subset('U', coord_def={X: x<1})
sage: X.restrict(U).coord_range()
x: (-oo, 1); y: (-oo, +oo)
}}}
The lower bound for `x` should be `O`, not `-oo`, and `y` should appear
as a periodic coordinate, i.e. one should get
{{{
sage: X.restrict(U).coord_range()
x: (0, 1); y: [0, 2] (periodic)
}}}

Sage <= 9.3 was free of this bug. In Sage >= 9.4, one can trace it to
the optional argument `bounds` of `RealChart.__init__`, which is used in
`RealChart.restrict` (cf. the line `res = type(self)(...,
bounds=self._bounds, ...)`)
and which is not correctly transmitted by `Chart.__classcall__`.

URL: https://trac.sagemath.org/32929
Reported by: egourgoulhon
Ticket author(s): Eric Gourgoulhon
Reviewer(s): Travis Scrimshaw

src/sage/manifolds/chart.py

@@ -200,7 +200,7 @@ class Chart(UniqueRepresentation, SageObject):
         sage: N = Manifold(2, 'N', field='complex', structure='topological')
         sage: XN.<Z1,Z2> = N.chart('Z1:period=1+2*I Z2')
         sage: XN.periods()
-        {0: 2*I + 1}
+        (2*I + 1, None)
 
     Coordinates are Sage symbolic variables (see
     :mod:`sage.symbolic.expression`)::
@@ -309,22 +309,27 @@ def __classcall__(cls, domain, coordinates='',
                 for x in names:
                     coordinates += x + ' '
                 coordinates = coordinates[:-1]
-            coordinates, coordinate_options = cls._parse_coordinates(domain, coordinates)
+            coordinates, parsed_options = cls._parse_coordinates(domain,
+                                                                 coordinates)
+            if not coordinate_options:
+                coordinate_options = parsed_options
 
         coord_string = ' '.join(str(x) for x in coordinates)
 
         try:
             return domain._charts_by_coord[coord_string]
         except KeyError:
             # Make coord_restrictions hashable
-            coord_restrictions = cls._normalize_coord_restrictions(coordinates, coord_restrictions)
+            coord_restrictions = cls._normalize_coord_restrictions(coordinates,
+                                                                   coord_restrictions)
             self = super().__classcall__(cls, domain, coordinates, calc_method,
                                          coord_restrictions=coord_restrictions,
                                          **coordinate_options)
             domain._charts_by_coord[coord_string] = self
             return self
 
-    def __init__(self, domain, coordinates, calc_method=None, periods=None, coord_restrictions=None):
+    def __init__(self, domain, coordinates, calc_method=None, periods=None,
+                 coord_restrictions=None):
         r"""
         Construct a chart.
 
@@ -364,13 +369,7 @@ def __init__(self, domain, coordinates, calc_method=None, periods=None, coord_re
         self.simplify = self._calc_method.simplify
 
         # Treatment of the coordinates:
-        if periods is None:
-            self._periods = {}
-        else:
-            # dictionary of periods (if any); key = coord. index
-            self._periods = {self._sindex + i: period
-                             for i, period in enumerate(periods)
-                             if period is not None}
+        self._periods = periods
 
         if len(coordinates) != self._manifold.dim():
             raise ValueError("the list of coordinates must contain " +
@@ -431,7 +430,8 @@ def _parse_coordinates(cls, domain, coordinates):
 
         - a tuple of variables (as elements of ``SR``)
         - a dictionary with possible keys:
-          - `"periods": a tuple of periods
+
+          - ``"periods"``: a tuple of periods
 
         TESTS::
 
@@ -504,7 +504,8 @@ def normalize(r):
         if coord_restrictions is None:
             return frozenset()
 
-        if callable(coord_restrictions) and not isinstance(coord_restrictions, Expression):
+        if callable(coord_restrictions) and not isinstance(coord_restrictions,
+                                                           Expression):
             # lambda-quoted
             coord_restrictions = coord_restrictions(*coordinates)
 
@@ -679,13 +680,12 @@ def manifold(self):
 
     def periods(self):
         r"""
-        Return the coordinate periods as a dictionary, possibly empty if no
-        coordinate is periodic.
+        Return the coordinate periods.
 
         OUTPUT:
 
-        - a dictionary with keys the indices of the periodic coordinates and
-          with values the periods.
+        - a tuple containing the period of each coordinate, with the
+          value ``None`` if the coordinate is not periodic
 
         EXAMPLES:
 
@@ -694,45 +694,30 @@ def periods(self):
             sage: M = Manifold(2, 'M', structure='topological')
             sage: X.<x,y> = M.chart()
             sage: X.periods()
-            {}
+            (None, None)
 
         Charts with a periodic coordinate::
 
             sage: Y.<u,v> = M.chart("u v:(0,2*pi):periodic")
             sage: Y.periods()
-            {1: 2*pi}
+            (None, 2*pi)
             sage: Z.<a,b> = M.chart(r"a:period=sqrt(2):\alpha b:\beta")
             sage: Z.periods()
-            {0: sqrt(2)}
-
-        The key in the output dictionary takes into account the index range
-        declared on the manifold with ``start_index``::
-
-            sage: M = Manifold(2, 'M', structure='topological', start_index=1)
-            sage: Y.<u,v> = M.chart("u v:(0,2*pi):periodic")
-            sage: Y[2]
-            v
-            sage: Y.periods()
-            {2: 2*pi}
-            sage: Z.<a,b> = M.chart(r"a:period=sqrt(2):\alpha b:\beta")
-            sage: Z[1]
-            a
-            sage: Z.periods()
-            {1: sqrt(2)}
+            (sqrt(2), None)
 
         Complex manifold with a periodic coordinate::
 
             sage: M = Manifold(2, 'M', field='complex', structure='topological',
             ....:              start_index=1)
             sage: X.<x,y> = M.chart("x y:period=1+I")
             sage: X.periods()
-            {2: I + 1}
+            (None, I + 1)
 
         TESTS::
 
             sage: M = Manifold(2, 'M', field=QQ, structure='topological')
             sage: X.<xq,yq> = M.chart(r"xq:period=3/2 yq:\zeta:period=2")
-            sage: X.periods()[0], X.periods()[1]
+            sage: X.periods()
             (3/2, 2)
 
         """
@@ -843,9 +828,11 @@ def restrict(self, subset, restrictions=None):
             for coord in self._xx:
                 coordinates += repr(coord) + ' '
             res_coord_restrictions = set(self._restrictions)
-            res_coord_restrictions.update(self._normalize_coord_restrictions(self._xx, restrictions))
+            res_coord_restrictions.update(self._normalize_coord_restrictions(self._xx,
+                                                                             restrictions))
             res = type(self)(subset, coordinates,
                              calc_method=self._calc_method._current,
+                             periods=self._periods,
                              # The coordinate restrictions are added
                              # to the result chart
                              coord_restrictions=res_coord_restrictions)
@@ -1781,7 +1768,7 @@ class RealChart(Chart):
         sage: c_spher1.<r,th,ph1> = \
         ....: V.chart(r'r:(0,+oo) th:(0,pi):\theta ph1:(0,2*pi):periodic:\phi_1')
         sage: c_spher1.periods()
-        {3: 2*pi}
+        (None, None, 2*pi)
         sage: c_spher1.coord_range()
         r: (0, +oo); th: (0, pi); ph1: [0, 2*pi] (periodic)
 
@@ -1791,7 +1778,7 @@ class RealChart(Chart):
         sage: c_spher2.<r,th,ph2> = \
         ....: V.chart(r'r:(0,+oo) th:(0,pi):\theta ph2:period=2*pi:\phi_2')
         sage: c_spher2.periods()
-        {3: 2*pi}
+        (None, None, 2*pi)
         sage: c_spher2.coord_range()
         r: (0, +oo); th: (0, pi); ph2: [0, 2*pi] (periodic)
 
@@ -1869,7 +1856,8 @@ class RealChart(Chart):
     :meth:`plot`.
 
     """
-    def __init__(self, domain, coordinates, calc_method=None, bounds=None, periods=None, coord_restrictions=None):
+    def __init__(self, domain, coordinates, calc_method=None, bounds=None,
+                 periods=None, coord_restrictions=None):
         r"""
         Construct a chart on a real topological manifold.
 
@@ -1910,6 +1898,14 @@ def _parse_coordinates(cls, domain, coordinates):
           character of the coordinate, and the (optional) coordinate
           LaTeX symbol
 
+        OUTPUT:
+
+        - a tuple of variables (as elements of ``SR``)
+        - a dictionary with possible keys:
+
+          - ``"periods"``: a tuple of periods
+          - ``"bounds"``: a tuple of coordinate ranges
+
         TESTS::
 
             sage: from sage.manifolds.chart import RealChart
@@ -2385,6 +2381,19 @@ def restrict(self, subset, restrictions=None):
             sage: a in A
             True
 
+        TESTS:
+
+        Check that :trac:`32929` is fixed::
+
+            sage: M = Manifold(2, 'M')
+            sage: X.<x,y> = M.chart(r"x:(0,+oo) y:(0,2):periodic")
+            sage: U = M.open_subset('U', coord_def={X: x<1})
+            sage: XU = X.restrict(U)
+            sage: XU.coord_range()
+            x: (0, 1); y: [0, 2] (periodic)
+            sage: XU.periods()
+            (None, 2)
+
         """
         if subset == self.domain():
             return self
@@ -2396,10 +2405,11 @@ def restrict(self, subset, restrictions=None):
             for coord in self._xx:
                 coordinates += repr(coord) + ' '
             res_coord_restrictions = set(self._restrictions)
-            res_coord_restrictions.update(self._normalize_coord_restrictions(self._xx, restrictions))
+            res_coord_restrictions.update(self._normalize_coord_restrictions(self._xx,
+                                                                             restrictions))
             res = type(self)(subset, coordinates,
                              calc_method=self._calc_method._current,
-                             bounds=self._bounds,
+                             bounds=self._bounds, periods=self._periods,
                              # The coordinate restrictions are added
                              # to the result chart and possibly
                              # transformed into coordinate bounds:

src/sage/manifolds/differentiable/chart.py

@@ -192,7 +192,7 @@ class DiffChart(Chart):
         sage: N = Manifold(2, 'N', field='complex')
         sage: XN.<Z1,Z2> = N.chart('Z1:period=1+2*I Z2')
         sage: XN.periods()
-        {0: 2*I + 1}
+        (2*I + 1, None)
 
     Coordinates are Sage symbolic variables (see
     :mod:`sage.symbolic.expression`)::
@@ -877,7 +877,7 @@ class RealDiffChart(DiffChart, RealChart):
         sage: c_spher1.<r,th,ph1> = \
         ....: V.chart(r'r:(0,+oo) th:(0,pi):\theta ph1:(0,2*pi):periodic:\phi_1')
         sage: c_spher1.periods()
-        {3: 2*pi}
+        (None, None, 2*pi)
         sage: c_spher1.coord_range()
         r: (0, +oo); th: (0, pi); ph1: [0, 2*pi] (periodic)
 
@@ -887,7 +887,7 @@ class RealDiffChart(DiffChart, RealChart):
         sage: c_spher2.<r,th,ph2> = \
         ....: V.chart(r'r:(0,+oo) th:(0,pi):\theta ph2:period=2*pi:\phi_2')
         sage: c_spher2.periods()
-        {3: 2*pi}
+        (None, None, 2*pi)
         sage: c_spher2.coord_range()
         r: (0, +oo); th: (0, pi); ph2: [0, 2*pi] (periodic)
 

src/sage/manifolds/manifold.py

@@ -669,7 +669,7 @@ def _an_element_(self):
             sage: p in V
             True
             sage: p.coord()
-            (-pi - 1, 0)
+            (-pi - 1, 2)
 
         """
         from sage.rings.infinity import Infinity

src/sage/manifolds/point.py

@@ -692,28 +692,14 @@ def __eq__(self, other):
             # raise ValueError("no common chart has been found to compare " +
             #                  "{} and {}".format(self, other))
         periods = common_chart.periods()
-        if periods:
-            # Special case of periodic coordinate(s):
-            ind = common_chart._sindex
-            for xs, xo in zip(self._coordinates[common_chart],
-                              other._coordinates[common_chart]):
-                diff = xs - xo
-                if ind in periods:
-                    period = periods[ind]
-                    if not (diff/period in ZZ):
-                        return False
-                else:
-                    if (isinstance(diff, Expression) and
-                        not diff.is_trivial_zero()):
-                        return False
-                    elif not (diff == 0):
-                        return False
-                ind += 1
-        else:
-            # Generic case:
-            for xs, xo in zip(self._coordinates[common_chart],
-                              other._coordinates[common_chart]):
-                diff = xs - xo
+        for ind, (xs, xo) in enumerate(zip(self._coordinates[common_chart],
+                                           other._coordinates[common_chart])):
+            diff = xs - xo
+            period = periods[ind]
+            if period is not None:
+                if not (diff/period in ZZ):
+                    return False
+            else:
                 if isinstance(diff, Expression) and not diff.is_trivial_zero():
                     return False
                 elif not (diff == 0):