Conversion problem between Laurent polynomial ring and its field of fractions

[soehms]

The issue is explained by the following example:

sage: R.<t> = LaurentPolynomialRing(ZZ)
sage: F = FractionField(R)
sage: R(F(1/t))
Traceback (most recent call last):
...
TypeError: fraction must have unit denominator

There is a natural conversion back to the Laurent polynomials. Most likely the denominator is not being checked if it is a unit in R.

[tscrim]

This is not a coercion, but a conversion (as it does not have to be defined going back to R).

[soehms]

The check for being a unit is performed in the section map of the natural inclusion of a ring into its ring of fractions. Thus, since this ring is not the Laurent polynomial ring itself but the corresponding polynomial ring, that test fails.

My attempt for a fix is to implement the conversion in the _element_constructor of the Laurent polynomial ring!

---

New commits:

​3716151

26425: initial

[SimonKing]

Note this:

sage: R.<t> = LaurentPolynomialRing(ZZ)
sage: t.is_unit()
True

So, things SHOULD work, as the denominator of 1/t is a unit. But:

sage: T = F(1/t)
sage: T.denominator().parent()
Univariate Polynomial Ring in t over Integer Ring
sage: R.fraction_field() is R.polynomial_ring().fraction_field()
True

Isn't that a bug? EDIT: No, because mathematically the two fraction fields are isomorphic.

Actually I recall that a couple of years ago I tried to let FractionField(R) be the LaurentSeriesRing, which mathematically is correct. EDIT: ... correct in the same way as the current fraction field is correct.

I think the following patch is not optimal:

+            P = x.parent()
+            if P.ring() == self.polynomial_ring():
+                d = self(x.denominator())
+                if not d.is_unit():
+                     raise TypeError("fraction must have unit denominator")
+                return self(x.numerator()) * d.inverse_of_unit()

I think we don't want to test that P.ring() == self.polynomial_ring(), but it is enough that self.polynomial_ring().has_coerce_map_from(P.ring()).

Or even better: We are talking about conversion here. So, there is no need to test whether the denominator coerces into self. We could just do

+        elif isinstance(x, FractionFieldElement):
+            # since the field of fraction of self is defined corresponding to the polynomial ring of self
+            # the conversion of its elements back must be treated separately (:trac:`26425`).
+            d = self(x.denominator())
+            if not d.is_unit():
+                 raise TypeError("fraction must have unit denominator")
+            return self(x.numerator()) * d.inverse_of_unit()

[git]

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

​2058e65

26425: correction accordng to reviewers suggestion

[soehms]

Replying to SimonKing:

Or even better: We are talking about conversion here. So, there is no need to test whether the denominator coerces into self. We could just do

+        elif isinstance(x, FractionFieldElement):
+            # since the field of fraction of self is defined corresponding to the polynomial ring of self
+            # the conversion of its elements back must be treated separately (:trac:`26425`).
+            d = self(x.denominator())
+            if not d.is_unit():
+                 raise TypeError("fraction must have unit denominator")
+            return self(x.numerator()) * d.inverse_of_unit()

As we talked before, the check was just matter of caution. I agree to have that removed!

[vdelecroix]

You need two : at the end of this line

+        Check that conversion back from fraction field does work (:trac:`26425`):

[SimonKing]

I fixed the missing ::.

All but one tests pass. The failing test is

king@klap:~/Sage/git/sage$ ./sage -t --warn-long 47.6 src/doc/common/conf.py  # 1 doctest failed

Running doctests with ID 2019-07-25-00-01-10-c2c7af32.
Git branch: t/26425/conversion_problem_laurent_fraction_field_26425
Using --optional=build,ccache,dochtml,frobby,gap_packages,gdb,gfortran,meataxe,memlimit,mpir,p_group_cohomology,python2,sage
Doctesting 1 file.
sage -t --warn-long 47.6 src/doc/common/conf.py
**********************************************************************
File "src/doc/common/conf.py", line 662, in doc.common.conf.call_intersphinx
Failed example:
    for line in open(thematic_index).readlines():  # optional - dochtml
        if "padics" in line:
            _ = sys.stdout.write(line)
Expected:
    <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sage-rings-padics-tutorial" title="(in Sage Reference Manual: p-Adics v...)"><span>Introduction to the p-adics</span></a></li>
Got:
    <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sage-rings-padics-tutorial" title="(in Sage Reference Manual: p-Adics v8.6.rc1)"><span>Introduction to the -adics</span></a></li>
**********************************************************************
1 item had failures:
   1 of   4 in doc.common.conf.call_intersphinx
    [3 tests, 1 failure, 0.01 s]
----------------------------------------------------------------------
sage -t --warn-long 47.6 src/doc/common/conf.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 0.1 seconds
    cpu time: 0.0 seconds
    cumulative wall time: 0.0 seconds

Is that related? Probably not. But before I give a positive review, I'd like to be sure that the error has not been introduced by this ticket.

I added reviewer and author names. Please check for misspellings.

---

New commits:

​38ef901

Fix a typo in the docs

[SimonKing]

Indeed:

king@klap:~/Sage/git/sage$ git checkout 8.9.beta3
HEAD is now at a1e1a8f... Updated SageMath version to 8.9.beta3
king@klap:~/Sage/git/sage$ ./sage -b
...
king@klap:~/Sage/git/sage$ ./sage -t --warn-long 47.6 src/doc/common/conf.py
Running doctests with ID 2019-07-25-00-12-53-64443448.
Git branch: HEAD
Using --optional=build,ccache,dochtml,frobby,gap_packages,gdb,gfortran,meataxe,memlimit,mpir,p_group_cohomology,python2,sage
Doctesting 1 file.
sage -t --warn-long 47.6 src/doc/common/conf.py
**********************************************************************
File "src/doc/common/conf.py", line 662, in doc.common.conf.call_intersphinx
Failed example:
    for line in open(thematic_index).readlines():  # optional - dochtml
        if "padics" in line:
            _ = sys.stdout.write(line)
Expected:
    <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sage-rings-padics-tutorial" title="(in Sage Reference Manual: p-Adics v...)"><span>Introduction to the p-adics</span></a></li>
Got:
    <li><a class="reference external" href="../reference/padics/sage/rings/padics/tutorial.html#sage-rings-padics-tutorial" title="(in Sage Reference Manual: p-Adics v8.6.rc1)"><span>Introduction to the -adics</span></a></li>
**********************************************************************
1 item had failures:
   1 of   4 in doc.common.conf.call_intersphinx
    [3 tests, 1 failure, 0.01 s]
----------------------------------------------------------------------
sage -t --warn-long 47.6 src/doc/common/conf.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 0.1 seconds
    cpu time: 0.0 seconds
    cumulative wall time: 0.0 seconds

Since this error has not been introduced by this ticket, I think I can give a positive review. Unless Vincent or someone else disagrees.

[soehms]

Thanks a lot, Simon!