From bb5ba2049a670be2fcaa0f253a8f197858fbd9fa Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Thu, 9 May 2024 12:57:55 -0600 Subject: [PATCH 01/21] Add some gbuild files to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 41494785183..444127cf593 100644 --- a/.gitignore +++ b/.gitignore @@ -204,6 +204,7 @@ build/pkgs/wheel/version_requirements.txt /pkgs/*/MANIFEST /pkgs/*/*.egg-info /pkgs/*/.tox +/pkgs/*/requirements-editable.txt /pkgs/sage-conf_pypi/sage_root/config.log /pkgs/sage-conf_pypi/sage_root/config.status From a0054521fe8b542cbab01e7e244ad89cd18e6333 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Thu, 9 May 2024 14:06:03 -0600 Subject: [PATCH 02/21] Implement __eq__ for curve divisors --- src/sage/schemes/generic/divisor.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/sage/schemes/generic/divisor.py b/src/sage/schemes/generic/divisor.py index c6d99b12204..daaa11feced 100644 --- a/src/sage/schemes/generic/divisor.py +++ b/src/sage/schemes/generic/divisor.py @@ -217,6 +217,24 @@ def _repr_(self): # generator being in front of the second one terms.sort(key=lambda x: x[1], reverse=True) return repr_lincomb([("V(%s)" % v, c) for c,v in terms]) + + def __eq__(self, other) -> bool: + """ + Check if the two divisors are on the same curve, and if so, check that their difference is zero. + + EXAMPLES:: + + sage: E = EllipticCurve([1, 2]) + sage: P = E(-1, 0) + sage: Q = E(1, 2) + sage: Pd = E.divisor(P) + sage: Qd = E.divisor(Q) + sage: Pd + Qd == Qd + Pd + True + sage: Pd != Qd + True + """ + return type(self) is type(other) and self.parent() == other.parent() and (self - other).is_zero() def scheme(self): """ From 8138a05c5266480e5c120b6900f790fb28ba506c Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Thu, 9 May 2024 16:00:48 -0600 Subject: [PATCH 03/21] Undo .gitignore changes --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index 444127cf593..41494785183 100644 --- a/.gitignore +++ b/.gitignore @@ -204,7 +204,6 @@ build/pkgs/wheel/version_requirements.txt /pkgs/*/MANIFEST /pkgs/*/*.egg-info /pkgs/*/.tox -/pkgs/*/requirements-editable.txt /pkgs/sage-conf_pypi/sage_root/config.log /pkgs/sage-conf_pypi/sage_root/config.status From 20671728cc2942a939b2b08c4a2dc4ece829bcbe Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Fri, 10 May 2024 14:22:19 -0600 Subject: [PATCH 04/21] Fix reduce in FormalSum --- src/sage/schemes/generic/divisor.py | 70 +++++++++++++---------------- src/sage/structure/formal_sum.py | 15 +++++-- 2 files changed, 43 insertions(+), 42 deletions(-) diff --git a/src/sage/schemes/generic/divisor.py b/src/sage/schemes/generic/divisor.py index daaa11feced..3c66525c412 100644 --- a/src/sage/schemes/generic/divisor.py +++ b/src/sage/schemes/generic/divisor.py @@ -24,13 +24,13 @@ sage: D1.parent() is D2.parent() True sage: D = D1 - D2 + D3; D - 3*(x, y) - (x, z) + 10*(x + 2*z, y + z) + 10*(x + 2*z, y + z) + 3*(x, y) - (x, z) sage: D[1][0] - -1 + 3 sage: D[1][1] - Ideal (x, z) of Multivariate Polynomial Ring in x, y, z over Finite Field of size 5 + Ideal (x, y) of Multivariate Polynomial Ring in x, y, z over Finite Field of size 5 sage: C.divisor([(3, pts[0]), (-1, pts[1]), (10, pts[5])]) - 3*(x, y) - (x, z) + 10*(x + 2*z, y + z) + 10*(x + 2*z, y + z) + 3*(x, y) - (x, z) """ #******************************************************************************* # Copyright (C) 2010 Volker Braun @@ -45,6 +45,7 @@ from sage.misc.repr import repr_lincomb from sage.misc.search import search from sage.rings.integer_ring import ZZ +from sage.sets.set import Set from sage.structure.formal_sum import FormalSum from .morphism import is_SchemeMorphism @@ -180,6 +181,15 @@ def _latex_(self): '\\mathrm{V}\\left(x + 2 y\\right) + 4 \\mathrm{V}\\left(x\\right) - 5 \\mathrm{V}\\left(y\\right)' + sage: E = EllipticCurve([1, 2]) + sage: P = E(-1, 0) + sage: Q = E(1, 2) + sage: Pd = E.divisor(P) + sage: Qd = E.divisor(Q) + sage: Pd + Qd == Qd + Pd + True + sage: Pd != Qd + True """ # The code is copied from _repr_ with latex adjustments terms = list(self) @@ -218,24 +228,6 @@ def _repr_(self): terms.sort(key=lambda x: x[1], reverse=True) return repr_lincomb([("V(%s)" % v, c) for c,v in terms]) - def __eq__(self, other) -> bool: - """ - Check if the two divisors are on the same curve, and if so, check that their difference is zero. - - EXAMPLES:: - - sage: E = EllipticCurve([1, 2]) - sage: P = E(-1, 0) - sage: Q = E(1, 2) - sage: Pd = E.divisor(P) - sage: Qd = E.divisor(Q) - sage: Pd + Qd == Qd + Pd - True - sage: Pd != Qd - True - """ - return type(self) is type(other) and self.parent() == other.parent() and (self - other).is_zero() - def scheme(self): """ Return the scheme that this divisor is on. @@ -247,7 +239,7 @@ def scheme(self): sage: pts = C.rational_points(); pts [(0, 0), (2, 2), (2, 3), (3, 1), (3, 4)] sage: D = C.divisor(pts[0])*3 - C.divisor(pts[1]); D - 3*(x, y) - (x - 2, y - 2) + -(x - 2, y - 2) + 3*(x, y) sage: D.scheme() Affine Plane Curve over Finite Field of size 5 defined by -x^9 + y^2 - x """ @@ -289,7 +281,7 @@ class Divisor_curve(Divisor_generic): sage: E.divisor([P, P]) 2*(x, y) sage: E.divisor([(3,P), (-4,5*P)]) - 3*(x, y) - 4*(x - 1/4*z, y + 5/8*z) + -4*(x - 1/4*z, y + 5/8*z) + 3*(x, y) """ def __init__(self, v, parent=None, check=True, reduce=True): """ @@ -381,39 +373,41 @@ def _repr_(self): """ return repr_lincomb([(tuple(I.gens()), c) for c, I in self]) - def support(self): + def support(self) -> Set: """ Return the support of this divisor, which is the set of points that occur in this divisor with nonzero coefficients. EXAMPLES:: - sage: x,y = AffineSpace(2, GF(5), names='xy').gens() + sage: A = AffineSpace(2, GF(5), names='xy') + sage: x, y = A.gens() sage: C = Curve(y^2 - x^9 - x) sage: pts = C.rational_points(); pts [(0, 0), (2, 2), (2, 3), (3, 1), (3, 4)] sage: D = C.divisor_group()([(3, pts[0]), (-1, pts[1])]); D - 3*(x, y) - (x - 2, y - 2) - sage: D.support() - [(0, 0), (2, 2)] + -(x - 2, y - 2) + 3*(x, y) + sage: D.support() == {A(0, 0), A(2, 2)} + True TESTS: This checks that :issue:`10732` is fixed:: sage: R. = GF(5)[] + sage: PS = ProjectiveSpace(R) sage: C = Curve(x^7 + y^7 + z^7) sage: pts = C.rational_points() sage: D = C.divisor([(2, pts[0])]) - sage: D.support() - [(0 : 4 : 1)] - sage: (D + D).support() - [(0 : 4 : 1)] + sage: D.support() == {PS(0, 4, 1)} + True + sage: (D + D).support() == {PS(0, 4, 1)} + True sage: E = C.divisor([(-3, pts[1]), (1, pts[2])]) - sage: (D - 2*E).support() - [(0 : 4 : 1), (1 : 2 : 1), (2 : 1 : 1)] + sage: (D - 2*E).support() == {PS(0, 4, 1), PS(1, 2, 1), PS(2, 1, 1)} + True sage: (D - D).support() - [] + {} """ try: return self._support @@ -427,7 +421,7 @@ def support(self): # rational points (see trac #16225) self._points = [(m, self.scheme().ambient_space().subscheme(p).rational_points()[0]) for (m, p) in self] pts = self._points - self._support = [s[1] for s in pts] + self._support = Set(s[1] for s in pts) return self._support def coefficient(self, P): @@ -444,7 +438,7 @@ def coefficient(self, P): sage: D.coefficient(pts[0]) 1 sage: D = C.divisor([(3, pts[0]), (-1, pts[1])]); D - 3*(x, y) - (x - 2, y - 2) + -(x - 2, y - 2) + 3*(x, y) sage: D.coefficient(pts[0]) 3 sage: D.coefficient(pts[1]) diff --git a/src/sage/structure/formal_sum.py b/src/sage/structure/formal_sum.py index f8f9b4d16de..b3a26db93ac 100644 --- a/src/sage/structure/formal_sum.py +++ b/src/sage/structure/formal_sum.py @@ -67,7 +67,6 @@ from sage.misc.repr import repr_lincomb import operator -from collections import OrderedDict from sage.modules.module import Module from sage.structure.element import ModuleElement @@ -93,7 +92,7 @@ def __init__(self, x, parent=None, check=True, reduce=True): coefficients into base ring, which can speed up constructing a formal sum. - ``reduce`` -- reduce (default: ``True``) if ``False``, do not - combine common terms + combine common terms. WARNING: setting this to ``False`` can cause commutativity issues when comparing sums that are equal but in different orders. EXAMPLES:: @@ -292,14 +291,22 @@ def reduce(self): sage: a 0 """ - new = OrderedDict() + new = dict() for coeff, x in self: try: coeff += new[x] except KeyError: pass new[x] = coeff - self._data = [(c, x) for (x, c) in new.items() if c] + + # We sort based on the string representation because some types have + # comparison operators that aren't total orders. There is nothing + # special about sorting based on the string representations, we just + # need to have some 'canonical representative' for comparisons + # to make sense, in particular for commutativity. + # Note however that some tests assume this ordering, and will + # need to be updated if the sorting here changes. + self._data = [(c, x) for (x, c) in sorted(new.items(), key=str) if c] class FormalSums(UniqueRepresentation, Module): From 3ccedc57ec373eeb28c4ee7f415d2af5a31aedcf Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Fri, 10 May 2024 14:33:01 -0600 Subject: [PATCH 05/21] Add _coerce_map_from_ in Divisor_generic --- src/sage/schemes/generic/divisor.py | 51 ++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/src/sage/schemes/generic/divisor.py b/src/sage/schemes/generic/divisor.py index 3c66525c412..d923ccabac6 100644 --- a/src/sage/schemes/generic/divisor.py +++ b/src/sage/schemes/generic/divisor.py @@ -122,6 +122,28 @@ def is_Divisor(x): class Divisor_generic(FormalSum): r""" A Divisor. + + TESTS:: + + sage: E = EllipticCurve([1, 2]) + sage: P = E(-1, 0) + sage: Q = E(1, 2) + sage: Pd = E.divisor(P) + sage: Qd = E.divisor(Q) + sage: Pd + Qd == Qd + Pd + True + sage: Pd != Qd + True + sage: C = EllipticCurve([2, 1]) + sage: R = C(1, 2) + sage: E = EllipticCurve([1, 2]) + sage: Q = E(1, 2) + sage: Qd = E.divisor(Q) + sage: Rd = C.divisor(R) + sage: Qd == Rd + False + sage: Rd == Qd + False """ def __init__(self, v, parent, check=True, reduce=True): @@ -181,15 +203,6 @@ def _latex_(self): '\\mathrm{V}\\left(x + 2 y\\right) + 4 \\mathrm{V}\\left(x\\right) - 5 \\mathrm{V}\\left(y\\right)' - sage: E = EllipticCurve([1, 2]) - sage: P = E(-1, 0) - sage: Q = E(1, 2) - sage: Pd = E.divisor(P) - sage: Qd = E.divisor(Q) - sage: Pd + Qd == Qd + Pd - True - sage: Pd != Qd - True """ # The code is copied from _repr_ with latex adjustments terms = list(self) @@ -227,6 +240,26 @@ def _repr_(self): # generator being in front of the second one terms.sort(key=lambda x: x[1], reverse=True) return repr_lincomb([("V(%s)" % v, c) for c,v in terms]) + + def _coerce_map_from_(self, other) -> bool: + r""" + Check if there is a coercion from ``other`` to ``self``. + This is used to prevent divisors on different schemes from comparing as equal to each other. + + EXAMPLES:: + + sage: C = EllipticCurve([2, 1]) + sage: R = C(1, 2) + sage: E = EllipticCurve([1, 2]) + sage: Q = E(1, 2) + sage: Qd = E.divisor(Q) + sage: Rd = C.divisor(R) + sage: Qd._coerce_map_from_(Rd) + False + sage: Rd._coerce_map_from_(Qd) + False + """ + return (isinstance(other, type(self)) and self.scheme() == other.scheme() and super()._coerce_map_from_(other)) def scheme(self): """ From 0c0f92b3b2c50ecf93ba18060f0abc72c52ee109 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Fri, 10 May 2024 16:47:56 -0600 Subject: [PATCH 06/21] Fix coercion --- src/sage/schemes/generic/divisor.py | 8 ++++---- src/sage/schemes/generic/divisor_group.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/sage/schemes/generic/divisor.py b/src/sage/schemes/generic/divisor.py index d923ccabac6..b06d3629c88 100644 --- a/src/sage/schemes/generic/divisor.py +++ b/src/sage/schemes/generic/divisor.py @@ -260,7 +260,7 @@ def _coerce_map_from_(self, other) -> bool: False """ return (isinstance(other, type(self)) and self.scheme() == other.scheme() and super()._coerce_map_from_(other)) - + def scheme(self): """ Return the scheme that this divisor is on. @@ -272,7 +272,7 @@ def scheme(self): sage: pts = C.rational_points(); pts [(0, 0), (2, 2), (2, 3), (3, 1), (3, 4)] sage: D = C.divisor(pts[0])*3 - C.divisor(pts[1]); D - -(x - 2, y - 2) + 3*(x, y) + -(x - 2, y - 2) + 3*(x, y) sage: D.scheme() Affine Plane Curve over Finite Field of size 5 defined by -x^9 + y^2 - x """ @@ -406,7 +406,7 @@ def _repr_(self): """ return repr_lincomb([(tuple(I.gens()), c) for c, I in self]) - def support(self) -> Set: + def support(self) -> frozenset: """ Return the support of this divisor, which is the set of points that occur in this divisor with nonzero coefficients. @@ -471,7 +471,7 @@ def coefficient(self, P): sage: D.coefficient(pts[0]) 1 sage: D = C.divisor([(3, pts[0]), (-1, pts[1])]); D - -(x - 2, y - 2) + 3*(x, y) + -(x - 2, y - 2) + 3*(x, y) sage: D.coefficient(pts[0]) 3 sage: D.coefficient(pts[1]) diff --git a/src/sage/schemes/generic/divisor_group.py b/src/sage/schemes/generic/divisor_group.py index 5dfd66a1bb9..622bde759b1 100644 --- a/src/sage/schemes/generic/divisor_group.py +++ b/src/sage/schemes/generic/divisor_group.py @@ -190,6 +190,22 @@ def _element_constructor_(self, x, check=True, reduce=True): else: return Divisor_generic([(self.base_ring()(1), x)], check=False, reduce=False, parent=self) + def _coerce_map_from_(self, other): + r""" + Check if there is a coercion from ``other`` to ``self``. + This is used to prevent divisors on different schemes from comparing as equal to each other. + + EXAMPLES:: + + sage: C = EllipticCurve([2, 1]) + sage: E = EllipticCurve([1, 2]) + sage: C.divisor_group()._coerce_map_from_(E.divisor_group()) + False + sage: E.divisor_group()._coerce_map_from_(C.divisor_group()) + False + """ + return (isinstance(other, type(self)) and self.scheme() == other.scheme() and super()._coerce_map_from_(other)) + def scheme(self): r""" Return the scheme supporting the divisors. From 2b464971a398d6883c0aa2cf97b2f9223fdeecdd Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Fri, 10 May 2024 16:49:12 -0600 Subject: [PATCH 07/21] Fix type annotation --- src/sage/schemes/generic/divisor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/schemes/generic/divisor.py b/src/sage/schemes/generic/divisor.py index b06d3629c88..2a5ddde6134 100644 --- a/src/sage/schemes/generic/divisor.py +++ b/src/sage/schemes/generic/divisor.py @@ -406,7 +406,7 @@ def _repr_(self): """ return repr_lincomb([(tuple(I.gens()), c) for c, I in self]) - def support(self) -> frozenset: + def support(self) -> Set: """ Return the support of this divisor, which is the set of points that occur in this divisor with nonzero coefficients. From aed1ba4f677b8591bf81cb6fdbbebf82770eebbd Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Mon, 13 May 2024 12:53:43 -0600 Subject: [PATCH 08/21] Change support to a list, fix tests --- src/sage/schemes/generic/divisor.py | 22 +++++++------- src/sage/schemes/generic/divisor_group.py | 6 +++- src/sage/structure/formal_sum.py | 35 ++++++++++++++--------- 3 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/sage/schemes/generic/divisor.py b/src/sage/schemes/generic/divisor.py index 2a5ddde6134..3dc8cbbda89 100644 --- a/src/sage/schemes/generic/divisor.py +++ b/src/sage/schemes/generic/divisor.py @@ -406,7 +406,7 @@ def _repr_(self): """ return repr_lincomb([(tuple(I.gens()), c) for c, I in self]) - def support(self) -> Set: + def support(self) -> list: """ Return the support of this divisor, which is the set of points that occur in this divisor with nonzero coefficients. @@ -420,8 +420,8 @@ def support(self) -> Set: [(0, 0), (2, 2), (2, 3), (3, 1), (3, 4)] sage: D = C.divisor_group()([(3, pts[0]), (-1, pts[1])]); D -(x - 2, y - 2) + 3*(x, y) - sage: D.support() == {A(0, 0), A(2, 2)} - True + sage: D.support() + [(0, 0), (2, 2)] TESTS: @@ -432,15 +432,15 @@ def support(self) -> Set: sage: C = Curve(x^7 + y^7 + z^7) sage: pts = C.rational_points() sage: D = C.divisor([(2, pts[0])]) - sage: D.support() == {PS(0, 4, 1)} - True - sage: (D + D).support() == {PS(0, 4, 1)} - True + sage: D.support() + [(0 : 4 : 1)] + sage: (D + D).support() + [(0 : 4 : 1)] sage: E = C.divisor([(-3, pts[1]), (1, pts[2])]) - sage: (D - 2*E).support() == {PS(0, 4, 1), PS(1, 2, 1), PS(2, 1, 1)} - True + sage: (D - 2*E).support() + [(2 : 1 : 1), (1 : 2 : 1), (0 : 4 : 1)] sage: (D - D).support() - {} + [] """ try: return self._support @@ -454,7 +454,7 @@ def support(self) -> Set: # rational points (see trac #16225) self._points = [(m, self.scheme().ambient_space().subscheme(p).rational_points()[0]) for (m, p) in self] pts = self._points - self._support = Set(s[1] for s in pts) + self._support = [s[1] for s in pts] return self._support def coefficient(self, P): diff --git a/src/sage/schemes/generic/divisor_group.py b/src/sage/schemes/generic/divisor_group.py index 622bde759b1..abe3843c110 100644 --- a/src/sage/schemes/generic/divisor_group.py +++ b/src/sage/schemes/generic/divisor_group.py @@ -195,7 +195,7 @@ def _coerce_map_from_(self, other): Check if there is a coercion from ``other`` to ``self``. This is used to prevent divisors on different schemes from comparing as equal to each other. - EXAMPLES:: + TESTS:: sage: C = EllipticCurve([2, 1]) sage: E = EllipticCurve([1, 2]) @@ -203,6 +203,10 @@ def _coerce_map_from_(self, other): False sage: E.divisor_group()._coerce_map_from_(C.divisor_group()) False + sage: E.divisor_group()._coerce_map_from_(E.divisor_group()) + True + sage: C.divisor_group()._coerce_map_from_(C.divisor_group()) + True """ return (isinstance(other, type(self)) and self.scheme() == other.scheme() and super()._coerce_map_from_(other)) diff --git a/src/sage/structure/formal_sum.py b/src/sage/structure/formal_sum.py index b3a26db93ac..6c4cca97e48 100644 --- a/src/sage/structure/formal_sum.py +++ b/src/sage/structure/formal_sum.py @@ -29,9 +29,9 @@ sage: -B -3*1/5 sage: A + B - 2/3 + 3*1/5 + 3*1/5 + 2/3 sage: A - B - 2/3 - 3*1/5 + -3*1/5 + 2/3 sage: B*3 9*1/5 sage: 2*A @@ -45,7 +45,7 @@ sage: loads(dumps(R)) == R True sage: a = R(2/3) + R(-5/7); a - 2/3 + -5/7 + -5/7 + 2/3 sage: loads(dumps(a)) == a True """ @@ -147,9 +147,9 @@ def __iter__(self): sage: for z in FormalSum([(1, 2), (-3, 7), (5, 1000)]): ....: print(z) + (5, 1000) (1, 2) (-3, 7) - (5, 1000) """ return iter(self._data) @@ -158,15 +158,15 @@ def __getitem__(self, n): EXAMPLES:: sage: v = FormalSum([(1, 2), (-3, 7), (5, 1000)]); v - 2 - 3*7 + 5*1000 + 5*1000 + 2 - 3*7 sage: v[0] - (1, 2) + (5, 1000) sage: v[1] - (-3, 7) + (1, 2) sage: v[2] - (5, 1000) + (-3, 7) sage: list(v) - [(1, 2), (-3, 7), (5, 1000)] + [(5, 1000), (1, 2), (-3, 7)] """ return self._data[n] @@ -196,7 +196,7 @@ def _latex_(self): EXAMPLES:: sage: latex(FormalSum([(1,2), (5, 8/9), (-3, 7)])) - 2 + 5\cdot \frac{8}{9} - 3\cdot 7 + 2 - 3\cdot 7 + 5\cdot \frac{8}{9} """ from sage.misc.latex import repr_lincomb symbols = [z[1] for z in self] @@ -229,6 +229,15 @@ def _richcmp_(self, other, op): True sage: a == 0 # 0 is coerced into a.parent()(0) False + + TESTS:: + + sage: a = FormalSum([(1,3),(2,5)]) + sage: b = FormalSum([(2,5),(1,3)]) + sage: a == b + True + sage: b == a + True """ return richcmp(self._data, other._data, op) @@ -246,7 +255,7 @@ def _add_(self, other): EXAMPLES:: sage: FormalSum([(1,3/7),(2,5/8)]) + FormalSum([(1,3/7),(-2,5)]) # indirect doctest - 2*3/7 + 2*5/8 - 2*5 + 2*3/7 - 2*5 + 2*5/8 """ return self.__class__(self._data + other._data, check=False, parent=self.parent()) @@ -298,7 +307,7 @@ def reduce(self): except KeyError: pass new[x] = coeff - + # We sort based on the string representation because some types have # comparison operators that aren't total orders. There is nothing # special about sorting based on the string representations, we just @@ -374,7 +383,7 @@ def _element_constructor_(self, x, check=True, reduce=True): sage: P = FormalSum([(1,2/3)]).parent() sage: P([(1,2/3), (5,-2/9)]) # indirect test - 2/3 + 5*-2/9 + 5*-2/9 + 2/3 """ if isinstance(x, FormalSum): P = x.parent() From d9e2706c2345dec9768581ceea900a21ca4d6618 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Mon, 13 May 2024 18:29:51 -0600 Subject: [PATCH 09/21] Apply suggestions from code review Co-authored-by: Travis Scrimshaw --- src/sage/schemes/generic/divisor.py | 20 -------------------- src/sage/structure/formal_sum.py | 11 ++++++++--- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/src/sage/schemes/generic/divisor.py b/src/sage/schemes/generic/divisor.py index 3dc8cbbda89..bc631182df0 100644 --- a/src/sage/schemes/generic/divisor.py +++ b/src/sage/schemes/generic/divisor.py @@ -241,26 +241,6 @@ def _repr_(self): terms.sort(key=lambda x: x[1], reverse=True) return repr_lincomb([("V(%s)" % v, c) for c,v in terms]) - def _coerce_map_from_(self, other) -> bool: - r""" - Check if there is a coercion from ``other`` to ``self``. - This is used to prevent divisors on different schemes from comparing as equal to each other. - - EXAMPLES:: - - sage: C = EllipticCurve([2, 1]) - sage: R = C(1, 2) - sage: E = EllipticCurve([1, 2]) - sage: Q = E(1, 2) - sage: Qd = E.divisor(Q) - sage: Rd = C.divisor(R) - sage: Qd._coerce_map_from_(Rd) - False - sage: Rd._coerce_map_from_(Qd) - False - """ - return (isinstance(other, type(self)) and self.scheme() == other.scheme() and super()._coerce_map_from_(other)) - def scheme(self): """ Return the scheme that this divisor is on. diff --git a/src/sage/structure/formal_sum.py b/src/sage/structure/formal_sum.py index 6c4cca97e48..7815644ed48 100644 --- a/src/sage/structure/formal_sum.py +++ b/src/sage/structure/formal_sum.py @@ -92,7 +92,12 @@ def __init__(self, x, parent=None, check=True, reduce=True): coefficients into base ring, which can speed up constructing a formal sum. - ``reduce`` -- reduce (default: ``True``) if ``False``, do not - combine common terms. WARNING: setting this to ``False`` can cause commutativity issues when comparing sums that are equal but in different orders. + combine common terms + + .. WARNING:: + + Setting ``reduce`` to ``False`` can cause issues when comparing + equal sums but with different orders and/or cancellations. EXAMPLES:: @@ -232,8 +237,8 @@ def _richcmp_(self, other, op): TESTS:: - sage: a = FormalSum([(1,3),(2,5)]) - sage: b = FormalSum([(2,5),(1,3)]) + sage: a = FormalSum([(1, 3), (2, 5)]) + sage: b = FormalSum([(2, 5), (1, 3)]) sage: a == b True sage: b == a From ff3e442ff096a0ecaf333fc00f2095d50b376073 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Tue, 14 May 2024 11:43:46 -0600 Subject: [PATCH 10/21] Fix for divisor coercion --- src/sage/schemes/generic/divisor.py | 7 ++++--- src/sage/schemes/generic/divisor_group.py | 10 +++++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/sage/schemes/generic/divisor.py b/src/sage/schemes/generic/divisor.py index bc631182df0..f4913e72cf1 100644 --- a/src/sage/schemes/generic/divisor.py +++ b/src/sage/schemes/generic/divisor.py @@ -136,14 +136,15 @@ class Divisor_generic(FormalSum): True sage: C = EllipticCurve([2, 1]) sage: R = C(1, 2) - sage: E = EllipticCurve([1, 2]) - sage: Q = E(1, 2) - sage: Qd = E.divisor(Q) sage: Rd = C.divisor(R) sage: Qd == Rd False sage: Rd == Qd False + sage: Qd == (2 * (Qd * 1/2)) + True + sage: Qd == 1/2 * Qd + False """ def __init__(self, v, parent, check=True, reduce=True): diff --git a/src/sage/schemes/generic/divisor_group.py b/src/sage/schemes/generic/divisor_group.py index abe3843c110..1704f0d7628 100644 --- a/src/sage/schemes/generic/divisor_group.py +++ b/src/sage/schemes/generic/divisor_group.py @@ -207,8 +207,16 @@ def _coerce_map_from_(self, other): True sage: C.divisor_group()._coerce_map_from_(C.divisor_group()) True + sage: D = 1/2 * E.divisor(E(1, 2)) + sage: D.parent()._coerce_map_from_(E.divisor_group()) + True + sage: E.divisor_group()._coerce_map_from_(D.parent()) + False """ - return (isinstance(other, type(self)) and self.scheme() == other.scheme() and super()._coerce_map_from_(other)) + if isinstance(other, DivisorGroup_generic): + return self.scheme() == other.scheme() and super()._coerce_map_from_(other) + else: + return super()._coerce_map_from_(other) def scheme(self): r""" From a547f1528683bc2add876c50a145907ce5df5ce9 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 15 May 2024 12:52:59 -0600 Subject: [PATCH 11/21] Update src/sage/structure/formal_sum.py Co-authored-by: Travis Scrimshaw --- src/sage/structure/formal_sum.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sage/structure/formal_sum.py b/src/sage/structure/formal_sum.py index 7815644ed48..efd49c32363 100644 --- a/src/sage/structure/formal_sum.py +++ b/src/sage/structure/formal_sum.py @@ -94,10 +94,10 @@ def __init__(self, x, parent=None, check=True, reduce=True): - ``reduce`` -- reduce (default: ``True``) if ``False``, do not combine common terms - .. WARNING:: + .. WARNING:: - Setting ``reduce`` to ``False`` can cause issues when comparing - equal sums but with different orders and/or cancellations. + Setting ``reduce`` to ``False`` can cause issues when comparing + equal sums but with different orders and/or cancellations. EXAMPLES:: From 3f4f7733c20f223fbe03dc4935337319022d74cd Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 15 May 2024 13:01:05 -0600 Subject: [PATCH 12/21] Fix _coerce_map_from_ in DivisorGroup_generic --- src/sage/schemes/generic/divisor_group.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/sage/schemes/generic/divisor_group.py b/src/sage/schemes/generic/divisor_group.py index 1704f0d7628..090c0300c16 100644 --- a/src/sage/schemes/generic/divisor_group.py +++ b/src/sage/schemes/generic/divisor_group.py @@ -213,10 +213,9 @@ def _coerce_map_from_(self, other): sage: E.divisor_group()._coerce_map_from_(D.parent()) False """ - if isinstance(other, DivisorGroup_generic): - return self.scheme() == other.scheme() and super()._coerce_map_from_(other) - else: - return super()._coerce_map_from_(other) + return (isinstance(other, DivisorGroup_generic) + and self.scheme() == other.scheme() + and super()._coerce_map_from_(other)) def scheme(self): r""" From 725427c9a45f78ff55b2a3a777f9d6e5a46b954c Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 15 May 2024 13:27:04 -0600 Subject: [PATCH 13/21] Change formal_sum comparison, revert changes to reduce --- src/sage/structure/formal_sum.py | 41 ++++++++++++++------------------ 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/src/sage/structure/formal_sum.py b/src/sage/structure/formal_sum.py index efd49c32363..5b433c3e7e6 100644 --- a/src/sage/structure/formal_sum.py +++ b/src/sage/structure/formal_sum.py @@ -29,9 +29,9 @@ sage: -B -3*1/5 sage: A + B - 3*1/5 + 2/3 + 2/3 + 3*1/5 sage: A - B - -3*1/5 + 2/3 + 2/3 - 3*1/5 sage: B*3 9*1/5 sage: 2*A @@ -45,7 +45,7 @@ sage: loads(dumps(R)) == R True sage: a = R(2/3) + R(-5/7); a - -5/7 + 2/3 + 2/3 + -5/7 sage: loads(dumps(a)) == a True """ @@ -67,6 +67,7 @@ from sage.misc.repr import repr_lincomb import operator +from collections import OrderedDict from sage.modules.module import Module from sage.structure.element import ModuleElement @@ -152,9 +153,9 @@ def __iter__(self): sage: for z in FormalSum([(1, 2), (-3, 7), (5, 1000)]): ....: print(z) - (5, 1000) (1, 2) (-3, 7) + (5, 1000) """ return iter(self._data) @@ -163,15 +164,15 @@ def __getitem__(self, n): EXAMPLES:: sage: v = FormalSum([(1, 2), (-3, 7), (5, 1000)]); v - 5*1000 + 2 - 3*7 + 2 - 3*7 + 5*1000 sage: v[0] - (5, 1000) - sage: v[1] (1, 2) - sage: v[2] + sage: v[1] (-3, 7) + sage: v[2] + (5, 1000) sage: list(v) - [(5, 1000), (1, 2), (-3, 7)] + [(1, 2), (-3, 7), (5, 1000)] """ return self._data[n] @@ -201,7 +202,7 @@ def _latex_(self): EXAMPLES:: sage: latex(FormalSum([(1,2), (5, 8/9), (-3, 7)])) - 2 - 3\cdot 7 + 5\cdot \frac{8}{9} + 2 + 5\cdot \frac{8}{9} - 3\cdot 7 """ from sage.misc.latex import repr_lincomb symbols = [z[1] for z in self] @@ -244,7 +245,9 @@ def _richcmp_(self, other, op): sage: b == a True """ - return richcmp(self._data, other._data, op) + self_data = [(c, x) for (x, c) in sorted(self._data, key=str)] + other_data = [(c, x) for (x, c) in sorted(other._data, key=str)] + return richcmp(self_data, other_data, op) def _neg_(self): """ @@ -260,7 +263,7 @@ def _add_(self, other): EXAMPLES:: sage: FormalSum([(1,3/7),(2,5/8)]) + FormalSum([(1,3/7),(-2,5)]) # indirect doctest - 2*3/7 - 2*5 + 2*5/8 + 2*3/7 + 2*5/8 - 2*5 """ return self.__class__(self._data + other._data, check=False, parent=self.parent()) @@ -305,22 +308,14 @@ def reduce(self): sage: a 0 """ - new = dict() + new = OrderedDict() for coeff, x in self: try: coeff += new[x] except KeyError: pass new[x] = coeff - - # We sort based on the string representation because some types have - # comparison operators that aren't total orders. There is nothing - # special about sorting based on the string representations, we just - # need to have some 'canonical representative' for comparisons - # to make sense, in particular for commutativity. - # Note however that some tests assume this ordering, and will - # need to be updated if the sorting here changes. - self._data = [(c, x) for (x, c) in sorted(new.items(), key=str) if c] + self._data = [(c, x) for (x, c) in new.items() if c] class FormalSums(UniqueRepresentation, Module): @@ -388,7 +383,7 @@ def _element_constructor_(self, x, check=True, reduce=True): sage: P = FormalSum([(1,2/3)]).parent() sage: P([(1,2/3), (5,-2/9)]) # indirect test - 5*-2/9 + 2/3 + 2/3 + 5*-2/9 """ if isinstance(x, FormalSum): P = x.parent() From 89945bd69f4f627ca2e3e30c3610a09f2e61f010 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 15 May 2024 13:33:52 -0600 Subject: [PATCH 14/21] Fix tests --- src/sage/schemes/generic/divisor.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/sage/schemes/generic/divisor.py b/src/sage/schemes/generic/divisor.py index f4913e72cf1..909f898c6e0 100644 --- a/src/sage/schemes/generic/divisor.py +++ b/src/sage/schemes/generic/divisor.py @@ -24,13 +24,13 @@ sage: D1.parent() is D2.parent() True sage: D = D1 - D2 + D3; D - 10*(x + 2*z, y + z) + 3*(x, y) - (x, z) + 3*(x, y) - (x, z) + 10*(x + 2*z, y + z) sage: D[1][0] - 3 + -1 sage: D[1][1] - Ideal (x, y) of Multivariate Polynomial Ring in x, y, z over Finite Field of size 5 + Ideal (x, z) of Multivariate Polynomial Ring in x, y, z over Finite Field of size 5 sage: C.divisor([(3, pts[0]), (-1, pts[1]), (10, pts[5])]) - 10*(x + 2*z, y + z) + 3*(x, y) - (x, z) + 3*(x, y) - (x, z) + 10*(x + 2*z, y + z) """ #******************************************************************************* # Copyright (C) 2010 Volker Braun @@ -253,7 +253,7 @@ def scheme(self): sage: pts = C.rational_points(); pts [(0, 0), (2, 2), (2, 3), (3, 1), (3, 4)] sage: D = C.divisor(pts[0])*3 - C.divisor(pts[1]); D - -(x - 2, y - 2) + 3*(x, y) + 3*(x, y) - (x - 2, y - 2) sage: D.scheme() Affine Plane Curve over Finite Field of size 5 defined by -x^9 + y^2 - x """ @@ -295,7 +295,7 @@ class Divisor_curve(Divisor_generic): sage: E.divisor([P, P]) 2*(x, y) sage: E.divisor([(3,P), (-4,5*P)]) - -4*(x - 1/4*z, y + 5/8*z) + 3*(x, y) + 3*(x, y) - 4*(x - 1/4*z, y + 5/8*z) """ def __init__(self, v, parent=None, check=True, reduce=True): """ @@ -400,7 +400,7 @@ def support(self) -> list: sage: pts = C.rational_points(); pts [(0, 0), (2, 2), (2, 3), (3, 1), (3, 4)] sage: D = C.divisor_group()([(3, pts[0]), (-1, pts[1])]); D - -(x - 2, y - 2) + 3*(x, y) + 3*(x, y) - (x - 2, y - 2) sage: D.support() [(0, 0), (2, 2)] @@ -419,7 +419,7 @@ def support(self) -> list: [(0 : 4 : 1)] sage: E = C.divisor([(-3, pts[1]), (1, pts[2])]) sage: (D - 2*E).support() - [(2 : 1 : 1), (1 : 2 : 1), (0 : 4 : 1)] + [(0 : 4 : 1), (1 : 2 : 1), (2 : 1 : 1)] sage: (D - D).support() [] """ @@ -452,7 +452,7 @@ def coefficient(self, P): sage: D.coefficient(pts[0]) 1 sage: D = C.divisor([(3, pts[0]), (-1, pts[1])]); D - -(x - 2, y - 2) + 3*(x, y) + 3*(x, y) - (x - 2, y - 2) sage: D.coefficient(pts[0]) 3 sage: D.coefficient(pts[1]) From e8f6a9cedc2495a52e325e9f3f4e1313b691ff83 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 15 May 2024 13:41:50 -0600 Subject: [PATCH 15/21] Formatting --- src/sage/structure/formal_sum.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/structure/formal_sum.py b/src/sage/structure/formal_sum.py index 5b433c3e7e6..93676d36fdf 100644 --- a/src/sage/structure/formal_sum.py +++ b/src/sage/structure/formal_sum.py @@ -383,7 +383,7 @@ def _element_constructor_(self, x, check=True, reduce=True): sage: P = FormalSum([(1,2/3)]).parent() sage: P([(1,2/3), (5,-2/9)]) # indirect test - 2/3 + 5*-2/9 + 2/3 + 5*-2/9 """ if isinstance(x, FormalSum): P = x.parent() From a6f3e0f25e3b5b94b245da29f7d11431b8f1390c Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 15 May 2024 16:41:59 -0600 Subject: [PATCH 16/21] Update src/sage/schemes/generic/divisor.py Co-authored-by: Travis Scrimshaw --- src/sage/schemes/generic/divisor.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sage/schemes/generic/divisor.py b/src/sage/schemes/generic/divisor.py index 909f898c6e0..1fe7770d5c6 100644 --- a/src/sage/schemes/generic/divisor.py +++ b/src/sage/schemes/generic/divisor.py @@ -45,7 +45,6 @@ from sage.misc.repr import repr_lincomb from sage.misc.search import search from sage.rings.integer_ring import ZZ -from sage.sets.set import Set from sage.structure.formal_sum import FormalSum from .morphism import is_SchemeMorphism From 6e9b4bb4c2cc3b53ba41c2af0cca81293a14793b Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 15 May 2024 16:42:45 -0600 Subject: [PATCH 17/21] Apply suggestions from code review Co-authored-by: Travis Scrimshaw --- src/sage/schemes/generic/divisor.py | 1 - src/sage/schemes/generic/divisor_group.py | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/sage/schemes/generic/divisor.py b/src/sage/schemes/generic/divisor.py index 1fe7770d5c6..fc2fb77a2a7 100644 --- a/src/sage/schemes/generic/divisor.py +++ b/src/sage/schemes/generic/divisor.py @@ -408,7 +408,6 @@ def support(self) -> list: This checks that :issue:`10732` is fixed:: sage: R. = GF(5)[] - sage: PS = ProjectiveSpace(R) sage: C = Curve(x^7 + y^7 + z^7) sage: pts = C.rational_points() sage: D = C.divisor([(2, pts[0])]) diff --git a/src/sage/schemes/generic/divisor_group.py b/src/sage/schemes/generic/divisor_group.py index 090c0300c16..9f29bbde0b9 100644 --- a/src/sage/schemes/generic/divisor_group.py +++ b/src/sage/schemes/generic/divisor_group.py @@ -192,8 +192,11 @@ def _element_constructor_(self, x, check=True, reduce=True): def _coerce_map_from_(self, other): r""" - Check if there is a coercion from ``other`` to ``self``. - This is used to prevent divisors on different schemes from comparing as equal to each other. + Return if there is a coercion map from ``other`` to ``self``. + + There is a coercion from another divisor group if the + schemes are equal and there is a coercion map from + the base rings. TESTS:: From d572a25730c1520e88847d1c509aa5818b921c22 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 15 May 2024 16:44:53 -0600 Subject: [PATCH 18/21] Coercion change --- src/sage/schemes/generic/divisor_group.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/schemes/generic/divisor_group.py b/src/sage/schemes/generic/divisor_group.py index 9f29bbde0b9..5c38d7d8589 100644 --- a/src/sage/schemes/generic/divisor_group.py +++ b/src/sage/schemes/generic/divisor_group.py @@ -217,7 +217,7 @@ def _coerce_map_from_(self, other): False """ return (isinstance(other, DivisorGroup_generic) - and self.scheme() == other.scheme() + and self.scheme().has_coerce_map_from(other.scheme()) and super()._coerce_map_from_(other)) def scheme(self): From c5385e1fd851d136452ba62df059aeadf1d7f045 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Wed, 15 May 2024 17:01:57 -0600 Subject: [PATCH 19/21] Update src/sage/schemes/generic/divisor_group.py Co-authored-by: Travis Scrimshaw --- src/sage/schemes/generic/divisor_group.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sage/schemes/generic/divisor_group.py b/src/sage/schemes/generic/divisor_group.py index 5c38d7d8589..e0db3b54c6b 100644 --- a/src/sage/schemes/generic/divisor_group.py +++ b/src/sage/schemes/generic/divisor_group.py @@ -194,8 +194,8 @@ def _coerce_map_from_(self, other): r""" Return if there is a coercion map from ``other`` to ``self``. - There is a coercion from another divisor group if the - schemes are equal and there is a coercion map from + There is a coercion from another divisor group if there is + a coercion map from the schemes and there is a coercion map from the base rings. TESTS:: From 82bbee4fd1382f4e7f485fdd19f67378b857ba70 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Thu, 16 May 2024 11:49:26 -0600 Subject: [PATCH 20/21] Update warning for setting reduce to False in FormalSum --- src/sage/structure/formal_sum.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/structure/formal_sum.py b/src/sage/structure/formal_sum.py index 93676d36fdf..cd81913134c 100644 --- a/src/sage/structure/formal_sum.py +++ b/src/sage/structure/formal_sum.py @@ -98,7 +98,7 @@ def __init__(self, x, parent=None, check=True, reduce=True): .. WARNING:: Setting ``reduce`` to ``False`` can cause issues when comparing - equal sums but with different orders and/or cancellations. + equal sums with different cancellations. EXAMPLES:: From ce4b05c80de172ba26b764081925783fc208da41 Mon Sep 17 00:00:00 2001 From: Vincent Macri Date: Thu, 16 May 2024 12:00:15 -0600 Subject: [PATCH 21/21] Clarify wording in reduce False warning --- src/sage/structure/formal_sum.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sage/structure/formal_sum.py b/src/sage/structure/formal_sum.py index cd81913134c..93f65c1f0e5 100644 --- a/src/sage/structure/formal_sum.py +++ b/src/sage/structure/formal_sum.py @@ -98,7 +98,8 @@ def __init__(self, x, parent=None, check=True, reduce=True): .. WARNING:: Setting ``reduce`` to ``False`` can cause issues when comparing - equal sums with different cancellations. + equal sums where terms are not combined in the same way (e.g. + `2x + 3x` and `4x + 1x` will compare as not equal). EXAMPLES::