Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

silently wrong result in coercion between InfinitePolynomialRing and LazyPowerSeriesRing #37756

Closed
mantepse opened this issue Apr 6, 2024 · 5 comments · Fixed by #37761
Closed

Comments

@mantepse
Copy link
Collaborator

mantepse commented Apr 6, 2024

Steps To Reproduce

sage: L.<x, y> = LazyPowerSeriesRing(QQ)
sage: R.<a> = InfinitePolynomialRing(QQ)
sage: o = L(0); r = a[0]
sage: o + r
y
sage: _.parent()
Infinite polynomial ring in a over Multivariate Lazy Taylor Series Ring in x, y over Rational Field

Expected Behavior

The result should either be a[0] (ideally with parent LazyPowerSeriesRing(InfinitePolynomialRing(QQ))) or an error should be raised.

Actual Behavior

A nonsense result,

Additional Information

The parent is explained as follows:

sage: cm.explain(o, r, op="add")
Coercion on left operand via
    Generic morphism:
      From: Multivariate Lazy Taylor Series Ring in x, y over Rational Field
      To:   Infinite polynomial ring in a over Multivariate Lazy Taylor Series Ring in x, y over Rational Field
Coercion on right operand via
    Coercion map:
      From: Infinite polynomial ring in a over Rational Field
      To:   Infinite polynomial ring in a over Multivariate Lazy Taylor Series Ring in x, y over Rational Field
Arithmetic performed after coercions.
Result lives in Infinite polynomial ring in a over Multivariate Lazy Taylor Series Ring in x, y over Rational Field
Infinite polynomial ring in a over Multivariate Lazy Taylor Series Ring in x, y over Rational Field

It is unclear to me, why we would want to have

sage: L.construction()[0]
Completion[('x', 'y'), prec=+Infinity]
sage: _.rank
4
sage: R.construction()[0]
InfPoly{[a], "lex", "dense"}
sage: _.rank
9.5
@mantepse
Copy link
Collaborator Author

mantepse commented Apr 6, 2024

In the end, the bug boils down to:

sage: L(a[0])
y

and within L._element_constructor_, this happens because

sage: P.<x,y> = QQ[]
sage: FF = P.fraction_field()
sage: FF(a[0])
y

@mantepse
Copy link
Collaborator Author

mantepse commented Apr 6, 2024

@nbruin, help greatly appreciated, of course.

@mantepse
Copy link
Collaborator Author

mantepse commented Apr 6, 2024

Making InfinitePolynomial_sparse.numerator() and denominator return elements of the InfinitePolynomialRing helps, but is not quite enough.

The InfinitePolynomialRing_sparse._element_constructor tries conversion to self._base (which is L below) of x._p (which is a_0 as an element of the multivariate polynomial ring), and this succeeds. To illustrate:

    sage: L.<x, y> = LazyPowerSeriesRing(QQ)
    sage: R.<a> = InfinitePolynomialRing(QQ)
    sage: M = InfinitePolynomialRing(L, names=["a"])
    sage: M(a[0])
    y

I'm not sure what to do about that. Note that this is exactly the coercion from R to M discovered.

@mantepse
Copy link
Collaborator Author

mantepse commented Apr 6, 2024

I forgot to realize (:-) that this has little to do with the LazyPowerSeriesRing:

sage: L.<x, y> = QQ[]
sage: R.<a> = InfinitePolynomialRing(QQ)
sage: M = InfinitePolynomialRing(L, names=["a"])
sage: c = a[0]
sage: M(c)
y

@mantepse
Copy link
Collaborator Author

mantepse commented Apr 6, 2024

I think the patch below might do the trick. I now get:

sage: L.<x, y> = LazyPowerSeriesRing(QQ)
sage: R.<a> = InfinitePolynomialRing(QQ)
sage: o = L(0); r = a[0]
sage: o + r
a_0
sage: _.parent()
Infinite polynomial ring in a over Multivariate Lazy Taylor Series Ring in x, y over Rational Field

However, I get an interesting error message here:

sage: sage: R.<a> = InfinitePolynomialRing(QQ)
sage: P.<x,y> = QQ[]
sage: FF = P.fraction_field()
sage: FF(a[0])  # correctly fails after providing numerator and denominator
---------------------------------------------------------------------------
...
RuntimeError: There is a bug in the coercion code in Sage.
Both x (=1) and y (=1) are supposed to have identical parents but they don't.
In fact, x has parent 'Infinite polynomial ring in a over Multivariate Polynomial Ring in x, y over Rational Field'
whereas y has parent 'Integer Ring'
Original elements 1 (parent Multivariate Polynomial Ring in x, y over Rational Field) and 1 (parent Infinite polynomial ring in a over Rational Field) and maps
<class 'sage.categories.morphism.SetMorphism'> (map internal to coercion system -- copy before use)
Generic morphism:
  From: Multivariate Polynomial Ring in x, y over Rational Field
  To:   Infinite polynomial ring in a over Multivariate Polynomial Ring in x, y over Rational Field
<class 'sage.structure.coerce_maps.DefaultConvertMap_unique'> (map internal to coercion system -- copy before use)
Coercion map:
  From: Infinite polynomial ring in a over Rational Field
  To:   Infinite polynomial ring in a over Multivariate Polynomial Ring in x, y over Rational Field

Any ideas what's going on?

diff --git a/src/sage/rings/polynomial/infinite_polynomial_element.py b/src/sage/rings/polynomial/infinite_polynomial_element.py
index 76bb926c17..87225b4804 100644
--- a/src/sage/rings/polynomial/infinite_polynomial_element.py
+++ b/src/sage/rings/polynomial/infinite_polynomial_element.py
@@ -560,6 +560,14 @@ class InfinitePolynomial(CommutativePolynomial, metaclass=InheritComparisonClass
         """
         return self._p.is_nilpotent()
 
+    def numerator(self):
+        P = self.parent()
+        return InfinitePolynomial(P, self._p.numerator())
+
+    def denominator(self):
+        P = self.parent()
+        return InfinitePolynomial(P, self._p.denominator())
+
     @cached_method
     def variables(self):
         """
diff --git a/src/sage/rings/polynomial/infinite_polynomial_ring.py b/src/sage/rings/polynomial/infinite_polynomial_ring.py
index a23b038731..169978acf9 100644
--- a/src/sage/rings/polynomial/infinite_polynomial_ring.py
+++ b/src/sage/rings/polynomial/infinite_polynomial_ring.py
@@ -922,7 +922,7 @@ class InfinitePolynomialRing_sparse(CommutativeRing):
             if isinstance(self._base, MPolynomialRing_polydict):
                 x = sage_eval(repr(), next(self.gens_dict()))
             else:
-                x = self._base(x)
+                x = self._base.coerce(x)
             # remark: Conversion to self._P (if applicable)
             # is done in InfinitePolynomial()
             return InfinitePolynomial(self, x)

vbraun pushed a commit to vbraun/sage that referenced this issue Aug 27, 2024
    
Fix sagemath#37756
    
URL: sagemath#37761
Reported by: Martin Rubey
Reviewer(s): Martin Rubey, Travis Scrimshaw
@vbraun vbraun closed this as completed in 0ec42d1 Sep 3, 2024
@mkoeppe mkoeppe added this to the sage-10.5 milestone Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants