added coefficient_ring #5429
added coefficient_ring #5429
Conversation
joschmitt
added
experimental
SoongNoonien
left a comment
SoongNoonien
left a comment
There was a problem hiding this comment.
This seems fine. I don't know that much about these action polynomials so I'm not entirely sure if we still want #5427 after this PR. Do we want base_ring and coefficient_ring to be effectively the same or do want them to be more in line with Nemocas/AbstractAlgebra.jl#2182? I would think that base_ring returning some internal universal polynomial ring isn't what we want, so #5427 should still be relevant but it wouldn't be needed to repair the tests of Nemocas/AbstractAlgebra.jl#2182 anymore.
|
|
||
| ##### Rings ##### | ||
|
|
||
| base_ring(apr::ActionPolyRing) = base_ring(__upr(apr)) |
There was a problem hiding this comment.
In fact I'd argue that "right" definition here would be
| base_ring(apr::ActionPolyRing) = base_ring(__upr(apr)) | |
| base_ring(apr::ActionPolyRing) = __upr(apr) |
Or rather: __upr should be replaced by base_ring. The "base ring" or a ring R is a ring from which R was constructed.
|
Yes, I agree on that. I wanted to wait for #5427 to be merged. I will set up a new PR for this now. |
4 participants
This PR adds
coefficient_ringand replaces the functionality ofbase_ringfor action polynomial rings as a follow-up to the discussion in #5427.