numerator() / denominator() for polynomials produce results with wrong parents

[maxale]

Steps To Reproduce

Consider the code:

sage: R.<x,y> = QQ[]
sage: x.parent()
Multivariate Polynomial Ring in x, y over Rational Field
sage: x.numerator()
x
sage: x.numerator().parent()
Multivariate Polynomial Ring in x, y over Integer Ring
sage: x.denominator()
1
sage: x.denominator().parent()
Integer Ring

Expected Behavior

To get numerator() or denominator() of an object t, we represent t as the irreducible fraction of elements from the same ring as t unless t comes from a fraction field. Note that QQ[] is NOT the fraction field of ZZ[]. Hence, numerator() or denominator() should not perform any lifting of the results to ZZ[] or to ZZ. That is,

• x.numerator() should be simply x over the same ring QQ not ZZ.

• x.denominator() should be R.one(), not ZZ.one().

Actual Behavior

The base ring is changed from QQ to ZZ by numerator(), while denominator() returns elements from ZZ rather than R.

Additional Information

No response

Environment

- **OS**: Ubuntu 22.04.4 LTS
- **Sage Version**: 10.3.beta7

Checklist

• [X] I have searched the existing issues for a bug report that matches the one I want to file, without success.
• [X] I have read the documentation and troubleshoot guide

[maxale]

For a bit more sophisticated base ring, numerator() works fine, but denominator() still has a wrong parent:

sage: F = PolynomialRing(QQ,1,'z').fraction_field()
sage: R.<x,y> = F[]
sage: x.parent()
Multivariate Polynomial Ring in x, y over Fraction Field of Multivariate Polynomial Ring in z over Rational Field
sage: x.numerator().parent()
Multivariate Polynomial Ring in x, y over Fraction Field of Multivariate Polynomial Ring in z over Rational Field
sage: x.denominator().parent()
Multivariate Polynomial Ring in z over Rational Field

[tscrim]

This isn't a bug. Please read the doc for the methods. While misleading for more algebraically minded people like myself, it does not refer to the corresponding fraction field. There is a big warning about this in the doc.

[maxale]

Ok, after reading the doc, I kind of understand what's happening with denominator, but behavior for .numerator() is still puzzling.

The doc for .numerator() says:

Return a numerator of self, computed as self * self.denominator().

but the following example shows that this is not the case:

sage: R.<x,y> = QQ[]
sage: x.parent()
Multivariate Polynomial Ring in x, y over Rational Field
sage: (x * x.denominator()).parent()
Multivariate Polynomial Ring in x, y over Rational Field
sage: x.numerator().parent()
Multivariate Polynomial Ring in x, y over Integer Ring

Here the parents of x * x.denominator() and x.numerator() differ. Why?

[tscrim]

Once we've cleared the denominators, we can write it in the smaller ring. So that's what we do by default. The multiplication by itself cannot do that automatically (nor should it try to do so as that would be expensive). Perhaps that doc could be made more precise with the output type.

[maxale]

I do not see why writing in the smaller ring is a good thing (in particular, when instead of a polynomial over the field as the original one, it produces a polynomial over just a ring).

What is worse, the current behavior is inconsistent with different inputs. Let me reiterate my second example:

sage: F = PolynomialRing(QQ,1,'z').fraction_field()
sage: R.<x,y> = F[]
sage: x.parent()
Multivariate Polynomial Ring in x, y over Fraction Field of Multivariate Polynomial Ring in z over Rational Field
sage: x.numerator().parent()
Multivariate Polynomial Ring in x, y over Fraction Field of Multivariate Polynomial Ring in z over Rational Field

Here we have x.numerator().parent() the same as x.parent(), which is R. Following the logic of fitting into a smaller ring, it should have the base ring of the polynomials in z (ie. PolynomialRing(QQ,1,'z')), not its fraction field.

So, the current behavior of .numerator() is neither documented, nor consistent across the inputs. This definitely requires a fix.

[tscrim]

On the contrary, there is nothing in the documentation that says it must be in the same parent, and in fact, you are cancelling denominators precisely in order to make the polynomial in the smaller ring. There is a coercion to the larger ring and the coefficients could then have "better" properties (for instance, a nontrivial prime factorization). The general rule of thumb is that the parent of the output should only depend on the parents of the inputs (NB: this is not a hard rule as there are often good reasons to not follow this). Having "consistency" (e.g., the same output type as the input) reduces the surprised factor a bit, but not necessary to change.

That being said, I would not be opposed to extending the QQ special case to all fraction field inputs. In addition, this behavior is documented (see in multi_polynomial_libsingular.pyx):

   If the base_field of self is the Rational Field then the
   numerator is a polynomial whose base_ring is the Integer Ring,
   this is done for compatibility to the univariate case.

[maxale]

First, the doc says "computed as self * self.denominator()", and I understand it literally. In reality, it returns not exactly self * self.denominator(), but something with a different parent, and I view it as an inaccuracy in the doc.

Second, under inconsistency I mean that if .numerator() aims to fit the result to a smaller ring, it should do so for all inputs, not just for polynomials over QQ (why make it an exception?). As I picture it now, what f.numerator() tries to/should do: compute d = f.denominator(), multiply the coefficients of f by d, and take the numerators of those (which will fit the coefficients to a smaller ring). This is my implementation for this approach:

def my_numerator(f):
    f *= f.denominator()
    return f.change_ring( f.lc().numerator().parent() )

Now, it works equally well for the first and second examples:

R.<x,y> = QQ[]
print( x.numerator().parent() )
print( my_numerator(x).parent() )

F = PolynomialRing(QQ,1,'z').fraction_field()
R.<x,y> = F[]
print( x.numerator().parent() )
print( my_numerator(x).parent() )

For polynomials over QQ, we have a matching outcome with the x.denominator():

Multivariate Polynomial Ring in x, y over Integer Ring Multivariate Polynomial Ring in x, y over Integer Ring

while for polynomials over F = PolynomialRing(QQ,1,'z').fraction_field() it does fit the result to a smaller ring, which is not done by x.numerator():

Multivariate Polynomial Ring in x, y over Fraction Field of Multivariate Polynomial Ring in z over Rational Field Multivariate Polynomial Ring in x, y over Multivariate Polynomial Ring in z over Rational Field

[tscrim]

First, the doc says "computed as self * self.denominator()", and I understand it literally. In reality, it returns not exactly self * self.denominator(), but something with a different parent, and I view it as an inaccuracy in the doc.

But it is accurate (mathematically speaking). At best it is an omission, but even for that, it is documented for the (currently one) case it does it. We wouldn't even care, or perhaps even notice, about this difference in a paper. You could argue it is imprecise, but it does do what it says it does. Anyways, this is moot from below.

Second, under inconsistency I mean that if .numerator() aims to fit the result to a smaller ring, it should do so for all inputs, not just for polynomials over QQ (why make it an exception?).

We are in agreement here. (I just making a caveat that this is not a must, but a should or good-thing-to-do.)

As I picture it now, what f.numerator() tries to/should do: compute d = f.denominator(), multiply the coefficients of f by d, and take the numerators of those (which will fit the coefficients to a smaller ring). This is my implementation for this approach:

def my_numerator(f):
    f *= f.denominator()
    return f.change_ring( f.lc().numerator().parent() )

This is what I was considering as well. +1