Skip to content

Conversation

@SoongNoonien
Copy link
Collaborator

This is a follow up to #2182 (the diff to #2182 shouldn't be breaking). In my quest to resolve #2172 I'm trying to make the universal polynomial ring as independent from multivariate polynomial rings as possible. This PR introduces a new function embed which maps multivariate polynomials into multivariate polynomials rings with more variables. This function should then be part of the ring interface for rings which are subject to the new universal ring.
The name embed is of course debatable.

In another follow up PR I'd like to introduce a new function called replace_gens (or similar) which takes a multivariate polynomial ring and a list of symbols and returns a new multivariate polynomial ring with the same properties (coefficient ring, ordering, etc.) but a different set of generators. With this function the universal polynomial ring should be independent of multivariate polynomial rings.

@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

❌ Patch coverage is 89.58333% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.95%. Comparing base (2980528) to head (ff85b7c).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/generic/UnivPoly.jl 87.17% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2192      +/-   ##
==========================================
- Coverage   87.95%   87.95%   -0.01%     
==========================================
  Files         127      127              
  Lines       31791    31790       -1     
==========================================
- Hits        27961    27960       -1     
  Misses       3830     3830              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thofma
Copy link
Member

thofma commented Oct 22, 2025

Nothing against this functionality, but this should not be exported. Also please give it a more obscure name, so that we do not accidentally make it available. Maybe _embed?

@SoongNoonien
Copy link
Collaborator Author

Why shouldn't it be exported? I think that this can be useful elsewhere. And I'm not sure if functions with obscure names should be part of an interface specification.

@thofma
Copy link
Member

thofma commented Oct 22, 2025

The embedding is not canonical/natural and people should use hom instead.

Comment on lines +1379 to +1382
Return the evaluation of the natural embedding of `parent(p)` into `R` at `p`.
This effectively replaces the $i$-th variable of `parent(p)` in `p` by the
$i$-th variable of `R`. For this to work, `R` needs to have at least as many
variables as `parent(p)`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Return the evaluation of the natural embedding of `parent(p)` into `R` at `p`.
This effectively replaces the $i$-th variable of `parent(p)` in `p` by the
$i$-th variable of `R`. For this to work, `R` needs to have at least as many
variables as `parent(p)`.
Return an element of `R` which is obtained from `p` by mapping the $i$-th variable
of `parent(p)` to the $i$-th variable of `R`.
For this to work, `R` needs to have at least as many variables as `parent(p)`.

@SoongNoonien
Copy link
Collaborator Author

Ok, I see. Using the hom implementation from OSCAR is even faster than this. @fingolfin suggested that this could be combined into a function called extend_gens (or similar) which takes a multivariate polynomial ring and a list of symbols and returns a new multivariate polynomial ring with the same properties as the old one but with the additional generators from the list of symbols. This alone would essentially be the function I proposed above as replace_gens but in addition to that extend_gens would return a map which plays the role of embed for the freshly created multivariate polynomial ring.
Apparently such a function is useful in multiple places in OSCAR, so this can have a proper name and be exported.

@thofma
Copy link
Member

thofma commented Oct 22, 2025

Is the plan to do this in OSCAR? The hom stuff does not exist in AbstractAlgebra. If the question does not make sense, I probably misunderstood the plan.

@SoongNoonien
Copy link
Collaborator Author

The plan is to do this in AbstractAlgebra. I'll try to explain the idea a bit more clearly.
I'd like to resolve #2172. To achieve this, I'd like to make the current implementation of the universal polynomial rings as mpoly agnostic as possible. Of course the polynomial specific bits like degree or leading_term will still depend on mpolys. These will later be implemented specifically for UniversalRing{MPoly}. But the basic arithmetic like addition and multiplication and the whole process of adding new variable/generators can be made mpoly agnostic. Let's call a ring type which can be universalized by UniversalRing a "generator ring type" (we need to find a better for this eventually but I can think of a better one at the moment). Then a generator ring type needs a gen/gens method which works analogously to the mpoly case, a symbols method which returns a vector of the generators and then two new functions, let's call them again embed and replace_gens (or extend_gens). The embed function defines how a ring with $n$ generators of a given generator ring type T is considered as a subring of a ring with $n+m$ generators of the same type T. The replace_gens (or extend_gens) function would take a ring of a given generator ring type T and return another ring of type T but with more generators.

Basically, one needs to be able to increase the number of generators and map elements to rings with more generators. There a various ways to implement this.

The problem I see with the hom stuff from OSCAR is that I'm not working with two fixed rings. I can happen that, for example, there are three rings involved, one with two generators, one with three and one with four. With the hom stuff I would need two morphisms to map all elements to the ring with four generators. The possible number of involved rings is not bounded. So, to make this work with hom I'd need to track all generator additions and store multiple morphisms.

@fingolfin
Copy link
Member

Ah but for the UniversalRing situation you describe, I think you need slightly different infrastructure: You don't want to addd rings; rather, you already have (in your example) three rings: A, B, and C, let's say with 4, 5 and 6 generators. Due to the way things work, we in fact know that B was obtained from A by adding one generator and likewise C was obtained from B that way.

And the "embedding" you apply strictly maps generators 1 to 4 of A to generators 1 to 4 of C.

We can't really afford to keep upgrade "maps" around here either. Where would they even be stored?

So here you really want a dedicated function (which then can be overloaded for the various types and also for optimization purposes) which "upgrades" an element this way... Maybe the old name upgrade isn't so bad after all for this internal function ;-)

@SoongNoonien
Copy link
Collaborator Author

Ah but for the UniversalRing situation you describe, I think you need slightly different infrastructure: You don't want to addd rings

Yes, I want to add rings. When one calls gens for a UniversalRing a new ring needs to be created with the additional generators.

So here you really want a dedicated function (which then can be overloaded for the various types and also for optimization purposes) which "upgrades" an element this way... Maybe the old name upgrade isn't so bad after all for this internal function ;-)

im_func from OSCAR as @fieker suggested should be fine here.

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.

Generalize the universal polynomial ring

3 participants