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

fix bad Frac(sparse polynomial ring over finite field) #37377

Merged
merged 3 commits into from
Mar 31, 2024

Conversation

mezzarobba
Copy link
Member

@mezzarobba mezzarobba commented Feb 17, 2024

In passing, add support for fraction fields of polynomial rings over prime fields based on NTL.

Fixes #37374

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few little details.

src/sage/rings/polynomial/polynomial_ring.py Outdated Show resolved Hide resolved
src/sage/rings/polynomial/polynomial_ring.py Outdated Show resolved Hide resolved
src/sage/rings/fraction_field_FpT.pyx Outdated Show resolved Hide resolved
src/sage/rings/polynomial/polynomial_modn_dense_ntl.pyx Outdated Show resolved Hide resolved
mezzarobba added a commit to mezzarobba/sage that referenced this pull request Feb 20, 2024
@mezzarobba
Copy link
Member Author

mezzarobba commented Feb 20, 2024 via email

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. You can set a positive review after these are changed.

@@ -2483,17 +2482,14 @@ def fraction_field(self):
sage: t(x)
x

Issue :issue:`37374`::
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Issue :issue:`37374`::
:issue:`37374`::

This is redundant, including in the compiled doc.

@@ -44,16 +44,27 @@ class FpT(FractionField_1poly_field):
"""
INPUT:

- ``R`` -- A polynomial ring over a finite field of prime order `p` with `2 < p < 2^16`
- ``R`` -- a dense polynomial ring over a finite field of prime order
`p` with `2 < p < 2^16`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`p` with `2 < p < 2^16`
`p` with `2 < p < 2^{16}`

Sorry, one more little thing to display properly.

@mezzarobba
Copy link
Member Author

mezzarobba commented Feb 20, 2024 via email

@tscrim
Copy link
Collaborator

tscrim commented Feb 20, 2024

No problem. Thank you.

@vbraun
Copy link
Member

vbraun commented Feb 21, 2024

sage -t --warn-long 38.3 --random-seed=123 src/sage/rings/polynomial/polynomial_modn_dense_ntl.pyx
**********************************************************************
File "src/sage/rings/polynomial/polynomial_modn_dense_ntl.pyx", line 878, in sage.rings.polynomial.polynomial_modn_dense_ntl.Polynomial_dense_modn_ntl_zz.__pow__
Failed example:
    (x-1)^(-5)
Expected:
    Traceback (most recent call last):
    ...
    TypeError: self must be an integral domain.
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "sage/structure/category_object.pyx", line 855, in sage.structure.category_object.CategoryObject.getattr_from_category
        return self._cached_methods[name]
    KeyError: 'fraction_field'
    <BLANKLINE>
    During handling of the above exception, another exception occurred:
    <BLANKLINE>
    Traceback (most recent call last):
      File "sage/structure/element.pyx", line 2744, in sage.structure.element.RingElement._div_
        frac = self._parent.fraction_field()
      File "sage/structure/category_object.pyx", line 849, in sage.structure.category_object.CategoryObject.__getattr__
        return self.getattr_from_category(name)
      File "sage/structure/category_object.pyx", line 864, in sage.structure.category_object.CategoryObject.getattr_from_category
        attr = getattr_from_other_class(self, cls, name)
      File "sage/cpython/getattr.pyx", line 357, in sage.cpython.getattr.getattr_from_other_class
        raise AttributeError(dummy_error_message)
    AttributeError: 'PolynomialRing_dense_mod_n_with_category' object has no attribute '_SageObject__custom_name'
    <BLANKLINE>
    During handling of the above exception, another exception occurred:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/home/release/Sage/src/sage/doctest/forker.py", line 712, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/release/Sage/src/sage/doctest/forker.py", line 1147, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.rings.polynomial.polynomial_modn_dense_ntl.Polynomial_dense_modn_ntl_zz.__pow__[11]>", line 1, in <module>
        (x-Integer(1))**(-Integer(5))
        ~~~~~~~~~~~~~^^~~~~~~~~~~~~~~
      File "sage/rings/polynomial/polynomial_modn_dense_ntl.pyx", line 917, in sage.rings.polynomial.polynomial_modn_dense_ntl.Polynomial_dense_modn_ntl_zz.__pow__
        return ~r
      File "sage/rings/polynomial/polynomial_element.pyx", line 1522, in sage.rings.polynomial.polynomial_element.Polynomial.__invert__
        return self.parent().one() / self
      File "sage/rings/polynomial/polynomial_element.pyx", line 2701, in sage.rings.polynomial.polynomial_element.Polynomial.__truediv__
        return (<Element>left)._div_(right)
      File "sage/structure/element.pyx", line 2746, in sage.structure.element.RingElement._div_
        raise bin_op_exception('/', self, other)
    TypeError: unsupported operand parent(s) for /: 'Univariate Polynomial Ring in x over Ring of integers modulo 100 (using NTL)' and 'Univariate Polynomial Ring in x over Ring of integers modulo 100 (using NTL)'
**********************************************************************
1 item had failures:
   1 of  13 in sage.rings.polynomial.polynomial_modn_dense_ntl.Polynomial_dense_modn_ntl_zz.__pow__
    [335 tests, 1 failure, 0.31 s]
----------------------------------------------------------------------
sage -t --warn-long 38.3 --random-seed=123 src/sage/rings/polynomial/polynomial_modn_dense_ntl.pyx  # 1 doctest failed
----------------------------------------------------------------------

mezzarobba added a commit to mezzarobba/sage that referenced this pull request Feb 23, 2024
@mezzarobba
Copy link
Member Author

File "src/sage/rings/polynomial/polynomial_modn_dense_ntl.pyx", line 878, in sage.rings.polynomial.polynomial_modn_dense_ntl.Polynomial_dense_modn_ntl_zz.pow
Failed example:
(x-1)^(-5)

This is with some additional commits merged in, right? Because I can't reproduce it with my branch alone. Anyway, I changed the test to ignore the exception test.

@vbraun
Copy link
Member

vbraun commented Mar 20, 2024

Merge conflict

The construction of the fraction field of a polynomial ring R over a
small finite fields tried ot use a class dedicated to the dense case
regardless whether R was dense or sparse, resulting in corrupted
elements represented as dense polynomials but with a parent set to a
sparse ring.

In passing, add support for fraction fields of polynomial rings over
prime fields based on NTL.

Fixes sagemath#37374
Copy link

Documentation preview for this PR (built with commit d75e9af; changes) is ready! 🎉

@vbraun vbraun merged commit a85b98f into sagemath:develop Mar 31, 2024
13 of 14 checks passed
@mkoeppe mkoeppe added this to the sage-10.4 milestone Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Corrupted polynomial (wrong results/exceptions, crash SIGABRT) obtained from sparse FractionField (FLINT)
4 participants