moving fraction_field method to categories #40213

fchapoton · 2025-06-05T15:39:37Z

out of the "ring.pyx" file and into the appropriate categories

On the way, one has to fix a few triggered issues. There remain several ones.

📝 Checklist

The title is concise and informative.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation and checked the documentation preview.

github-actions · 2025-06-05T16:32:14Z

Documentation preview for this PR (built with commit 8ade692; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

mantepse · 2025-06-06T07:36:57Z

src/sage/rings/lazy_series_ring.py

+    def ngens(self):
+        r"""
+        Return the number of generators of ``self``.
+
+        This is always 1.
+
+        EXAMPLES::
+
+            sage: L = LazyDirichletSeriesRing(ZZ, 'z')
+            sage: L.ngens()
+            1
+        """
+        return 1
+


Not sure whether this is a good idea - I don't think the Dirichlet series ring is finitely generated.

Well, no, but you have one generator t somewhere in the code..

Actually, we don't. There is a data structure, which we carefully hide - the _internal_poly_ring.

Why is adding ngens necessary or useful? Maybe it helps me to understand or we find a better solution.

Well, I am currently investigating the other failures.

I had to do that change in Lazy Dirichlet for their fraction field to work.

Here is the failure without my fix

File "src/sage/rings/lazy_series_ring.py", line 3916, in sage.rings.lazy_series_ring.LazyDirichletSeriesRing.__init__ Failed example: TestSuite(L).run() # needs sage.symbolic Expected nothing Got: Failure in _test_fraction_field: Traceback (most recent call last): File "sage/structure/category_object.pyx", line 857, in sage.structure.category_object.CategoryObject.getattr_from_category return self._cached_methods[name] KeyError: 'ngens' <BLANKLINE> During handling of the above exception, another exception occurred: <BLANKLINE> Traceback (most recent call last): File "/home/chapoton/sage/src/sage/misc/sage_unittest.py", line 298, in run test_method(tester=tester) File "/home/chapoton/sage/src/sage/categories/integral_domains.py", line 203, in _test_fraction_field fraction_field = self.fraction_field() ^^^^^^^^^^^^^^^^^^^^^ File "sage/misc/cachefunc.pyx", line 2333, in sage.misc.cachefunc.CachedMethodCallerNoArgs.__call__ self.cache = f(self._instance) File "/home/chapoton/sage/src/sage/categories/integral_domains.py", line 191, in fraction_field return sage.rings.fraction_field.FractionField_generic(self) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/chapoton/sage/src/sage/rings/fraction_field.py", line 205, in __init__ Parent.__init__(self, base=R, names=R._names, category=cat) File "sage/structure/parent.pyx", line 301, in sage.structure.parent.Parent.__init__ self._assign_names(names, normalize) File "sage/structure/parent_gens.pyx", line 152, in sage.structure.parent_gens.ParentWithGens._assign_names names = category_object.normalize_names(self.ngens(), names) File "/home/chapoton/sage/src/sage/rings/fraction_field.py", line 937, in ngens return self._R.ngens() ^^^^^^^^^^^^^ File "sage/structure/category_object.pyx", line 851, in sage.structure.category_object.CategoryObject.__getattr__ return self.getattr_from_category(name) File "sage/structure/category_object.pyx", line 866, in sage.structure.category_object.CategoryObject.getattr_from_category attr = getattr_from_other_class(self, cls, name) File "sage/cpython/getattr.pyx", line 358, in sage.cpython.getattr.getattr_from_other_class raise AttributeError(dummy_error_message) AttributeError: 'LazyDirichletSeriesRing_with_category' object has no attribute 'ngens' ------------------------------------------------------------ The following tests failed: _test_fraction_field

Do you know under which assumptions FractionField_generic is supposed to work?

Here is something that causes problems :

sage: L = LazyDirichletSeriesRing(ZZ,'z') sage: L._names ('z',)

Sorry, I don't understand. Why is this causing problems?

Well, this name is transmitted to the fraction field, which then tries to call "ngens", which forces us to add the silly "ngens" method

So I think for the moment, the simplest way out is just to add the silly method "ngens". I have more difficult issues to fix, in particular something about src/sage/algebras/splitting_algebra.py

mantepse · 2025-06-06T12:54:37Z

src/sage/combinat/sf/sf.py

            sage: Sym1 = SymmetricFunctions(FiniteField(23))
            sage: Sym2 = SymmetricFunctions(Integers(23))
-            sage: TestSuite(Sym).run()
+            sage: TestSuite(Sym).run(skip="_test_fraction_field")


Sym has a (very naive) implementation of a fraction field, which is actually used (in the lazy code). I missed what's going wrong, sorry.

fractions field of realizations are working fine. What is not working is the interplay between the Sym without realization and the fraction field tests.

fchapoton · 2025-07-03T09:55:58Z

undoign the changes in src/sage/rings/polynomial/polynomial_quotient_ring.py fixes the current failurs, but trigger new failures in padics.

sagemathgh-40749: small cleanup of the file splitting_algebra.py in preparation for sagemath#40213 ### 📝 Checklist - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. URL: sagemath#40749 Reported by: Frédéric Chapoton Reviewer(s): David Coudert

mantepse · 2025-09-15T06:39:07Z

could you convert this to draft until it is ready for review?

fchapoton · 2025-09-15T08:15:53Z

Voila. Help welcome, this is a difficult one.

mantepse · 2025-10-06T11:27:52Z

I would like to help, but I don't know how. I figured that,
old code:

sage: from sage.algebras.splitting_algebra import SplittingAlgebra
sage: L.<t, u, v, w > = LaurentPolynomialRing(ZZ); x = polygen(L)
sage: S = SplittingAlgebra(x^4 -t*x^3 - u*x^2 - v*x + w, ('X', 'Y', 'Z'), warning=False)
sage: S.fraction_field
<bound method CommutativeRing.fraction_field of Splitting Algebra of x^4 - t*x^3 - u*x^2 - v*x + w with roots [X, Y, Z, -Z - Y - X + t] over Multivariate Laurent Polynomial Ring in t, u, v, w over Integer Ring>

which eventually calls K = sage.rings.fraction_field.FractionField_generic(self)

whereas the new code

sage: S.fraction_field
<bound method PolynomialQuotientRing_generic.fraction_field of Splitting Algebra of x^4 - t*x^3 - u*x^2 - v*x + w with roots [X, Y, Z, -Z - Y - X + t] over Multivariate Laurent Polynomial Ring in t, u, v, w over Integer Ring>

which does something different:

        frac = self.base_ring().fraction_field()
        return self.base().change_ring(frac).quo(self.modulus(),
                                                 category=Fields())

fchapoton · 2025-10-06T11:38:41Z

yes, this is a tough case. At this point, I am wondering if one should implement _coerce_map_from_ for these splitting algebras. But the code is recursive and complicated and i do not understand it.

mantepse · 2025-10-06T16:31:32Z

Is this about splitting algebras (@soehms) or about the coercion system (@nbruin, @tscrim) ?

soehms · 2025-10-07T20:19:45Z

Is this about splitting algebras (@soehms) or about the coercion system (@nbruin, @tscrim) ?

I think the issue is due to the test case. It cannot be verified by code that S is an integral domain, because the test for whether the modulo is irreducible isn't implemented for this case:

sage: from sage.algebras.splitting_algebra import SplittingAlgebra
....: L.<t, u, v, w > = LaurentPolynomialRing(ZZ); x = polygen(L)
....: S = SplittingAlgebra(x^4 -t*x^3 - u*x^2 - v*x + w, ('X', 'Y', 'Z'), warning=False)
sage: S._defining_polynomial.is_irreducible()
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
...
NotImplementedError: cannot rewrite Factorization Algebra of x^4 - t*x^3 - u*x^2 - v*x + w with roots [X] over Multivariate Laurent Polynomial Ring in t, u, v, w over Integer Ring as an isomorphic ring

The warning that should be suppressed by warning=False is this:

            if warning:
                warn(f'Assuming {monic_polynomial} to have maximal Galois group!')
                warning = False  # one warning must be enough

Therefore, it is the user's responsibility to check whether S is an integral domain.

My suggestion is to add skip='_test_fraction_field' to the TestSuite run on S. Perhaps an additional example that doesn't require the skip argument could clarify that fraction fields are not impossible in general:

sage: B = S.base_ring().base_ring()
sage: type(B)
<class 'sage.algebras.splitting_algebra.SplittingAlgebra_with_category'>
sage: TestSuite(B).run()
sage:

or this one:

sage: x = polygen(ZZ)
sage: T = SplittingAlgebra(x^2+1, ('I',))
sage: TestSuite(T).run()

BTW: The warning message above is broken. It won't appear if you remove warning=False because it isn't passed to the recursive call. I'll open a PR to fix this later.

fchapoton · 2025-10-12T11:33:16Z

what about the failures in cubic Hecke algebras ? any suggestions ?

Failed example:: src/sage/algebras/hecke_algebras/cubic_hecke_algebra.py

soehms · 2025-10-15T06:27:24Z

what about the failures in cubic Hecke algebras ? any suggestions ?

Failed example:: src/sage/algebras/hecke_algebras/cubic_hecke_algebra.py

These failures are caused by the custom method fraction_field that you added to src/sage/rings/polynomial/polynomial_quotient_ring.py. They occur when the modulus of the quotient ring cannot be verified as irreducible. Without your branch, this raises a NotImplementedError, which is caught in solve_with_extensions->create_roots when attempting to obtain the splitting_field. With your branch, a TypeError is raised instead, which is not caught.

The simplest, but symptomatic, solution would be to simply add the TypeError to the exception in solve_with_extensions->create_roots. Closer to the previous behavior would be to have fraction_field raise a NotImplementedError in such a case, as before.

Therefore, I suggest either adding an assert(self.modulus().is_irreducible()) to the existing fraction_field method in src/sage/rings/polynomial/polynomial_quotient_ring.py or (if you find this too general) adding a custom fraction_field to src/sage/algebras/splitting_algebra.py, for example like this

    def fraction_field(self):
        assert(self.modulus().is_irreducible())
        return super().fraction_field()

fchapoton force-pushed the moving_fraction_field branch 2 times, most recently from e471125 to ab119ab Compare June 6, 2025 07:00

mantepse reviewed Jun 6, 2025

View reviewed changes

fchapoton added c: commutative algebra s: needs work labels Jun 6, 2025

github-actions bot added s: needs review and removed s: needs work labels Jun 26, 2025

moving fraction_field method to categories

b5f125b

fchapoton force-pushed the moving_fraction_field branch from 1a9e1ef to b5f125b Compare July 3, 2025 09:52

Merge branch 'develop' into moving_fraction_field

9170f34

fchapoton added s: needs work and removed s: needs review labels Jul 9, 2025

Merge branch 'develop' into moving_fraction_field

5704802

github-actions bot added s: needs review and removed s: needs work labels Aug 18, 2025

fchapoton added s: needs work and removed s: needs review labels Aug 27, 2025

fchapoton mentioned this pull request Aug 29, 2025

trying to get rid of some CommutativeRing #40703

Merged

2 tasks

Merge branch 'develop' into moving_fraction_field

e2c0e31

github-actions bot added s: needs review and removed s: needs work labels Aug 29, 2025

fchapoton mentioned this pull request Sep 1, 2025

small cleanup of the file splitting_algebra.py #40749

Merged

3 tasks

fchapoton added 2 commits September 7, 2025 14:05

Merge branch 'develop' into moving_fraction_field

fc62000

Merge branch 'develop' into moving_fraction_field

4bb2320

fchapoton marked this pull request as draft September 15, 2025 08:15

github-actions bot removed the s: needs review label Sep 15, 2025

fchapoton added 4 commits October 5, 2025 11:10

Merge branch 'develop' into moving_fraction_field

7cc1bc5

enhancing the category of quotient rings

2510d16

another fix

859bb34

some fixes

56c7f38

soehms mentioned this pull request Oct 9, 2025

Fix missing warning in SplittingAlgebra #41016

Open

5 tasks

Merge branch 'develop' into moving_fraction_field

241e2f8

Merge branch 'develop' into moving_fraction_field

8ade692

Uh oh!

Uh oh!

moving fraction_field method to categories #40213

Are you sure you want to change the base?

moving fraction_field method to categories #40213

Uh oh!

Conversation

fchapoton commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Checklist

Uh oh!

github-actions bot commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fchapoton commented Jul 3, 2025

Uh oh!

mantepse commented Sep 15, 2025

Uh oh!

fchapoton commented Sep 15, 2025

Uh oh!

mantepse commented Oct 6, 2025

Uh oh!

fchapoton commented Oct 6, 2025

Uh oh!

mantepse commented Oct 6, 2025

Uh oh!

soehms commented Oct 7, 2025

Uh oh!

fchapoton commented Oct 12, 2025

Uh oh!

soehms commented Oct 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fchapoton commented Jun 5, 2025 •

edited

Loading

github-actions bot commented Jun 5, 2025 •

edited

Loading