Caching for universal polynomial rings

[SoongNoonien]

While looking into #1979 I noticed that caching for universal polynomial rings is implemented in a weird way, which is incompatible with the ideas I have for #1979. I'm not sure if there even is a sensible way to implement caching in this case. As far as I understand, currently the caching for universal polynomial rings doesn't depend on the variables. So if I create a universal polynomial ring and add some variables to it and then create a new one, the new one will have the same variables as the old one. I my opinion this even contradicts the docstring of universal_polynomial_ring:

Given a base ring R, return an object representing
the universal polynomial ring $S = R[\ldots]$ with no variables in it initially

This is now caching looks in action:

julia> R=universal_polynomial_ring(QQ)
Universal Polynomial Ring over Rationals

julia> x=gen(R,:x)
x

julia> gens(R)
1-element Vector{AbstractAlgebra.Generic.UnivPoly{Rational{BigInt}}}:
 x

julia> S=universal_polynomial_ring(QQ)
Universal Polynomial Ring over Rationals

julia> gens(S)
1-element Vector{AbstractAlgebra.Generic.UnivPoly{Rational{BigInt}}}:
 x

julia> gen(S,:y)
y

julia> gens(R)
2-element Vector{AbstractAlgebra.Generic.UnivPoly{Rational{BigInt}}}:
 x
 y

Is there a use case for caching for universal polynomial rings?

[thofma]

While contradicting the documentation, I find the current caching behavior the most useful/sensible. If cached = true, then for every coefficient ring, there will only ever be one universal polynomial ring.

On the other hand, I don't use them often and would not mind changing/fixing it, if it makes other things easier.

[SoongNoonien]

Ok, suppose we had something like #1979. What would you expect the last command to return in the following example?

julia> R,(x,)=universal_polynomial_ring(QQ, [:x]);

julia> S,(x,y)=universal_polynomial_ring(QQ, [:x, :y]);

julia> gens(R)

Following the current logic this should return a vector with the entries x and y. But this would mean that constructing one polynomial ring can alter an already existing one.

[thofma]

In this case, I would expect that caching takes the variables supplied into account.

[lgoettgens]

In this case, I would expect that caching takes the variables supplied into account.

julia> R,(x,)=universal_polynomial_ring(QQ, [:x]);

julia> S,(x,y)=universal_polynomial_ring(QQ, [:x, :y]);

julia> gen(R, :y);

now R and S have the same variables. Should they now be the same or not? What if we swap lines 2 and 3, since then R is already identical to S at the time of creation of S.

[thofma]

maybe disable caching for universal polynomial rings

[lgoettgens]

I thought a bit about it during lunch. The only "nice" semantics of caching I could think of is delegating this kwarg to the mpoly rings that are used underneath.
So with cached=true, the univ poly ring are not cached, but the mpoly rings used underneath are. But I am not sure that this is even useful at all.

[SoongNoonien]

Interestingly this is done on construction but not when adding new variables.

[SoongNoonien]

Ok, one would need to add a new field to UniversalPolyRing which stores if the underlying mpoly rings should be cached.

[thofma]

No, this is not useful. The multivariate polynomial rings should never be cached. This is unrelated to what caching of the univariate polynomial ring means.

[SoongNoonien]

Why shouldn't they be cached? I think it is working quite well.

[thofma]

What doesn't work if they are not cached?

The cached argument for constructors should change the behavior of the function for users, such that they get identical results when calling the function with the same arguments. This test

@test universal_polynomial_ring(R; internal_ordering=ord, cached = true) === universal_polynomial_ring(R; internal_ordering=ord, cached = true)

which is removed in #1992 is the very definition of what cached = true means.

[SoongNoonien]

Ok, I see. But how can we fulfill this test if we disable caching for universal polynomial rings completely?

[thofma]

I think the easiest way (and backwards compatible) would be just to use the variables when caching. So extend UnivPolyID to include the variables and then

get_cached!(UnivPolyID, (R, internal_ordering, s), ...)

This way we get the properties we want for caching. It leads to some oddities, but at least it is consistent.

[fingolfin]

I don't think caching the underlying polynomial rings should be affected by the cached argument to the top level universal polynomial ring, in so far PR #1992 doesn't make sense to me.

The current caching logic for UniversalPolynomialRing works exactly as I would expect it to -- though I agree its effects are rather weird here, because of side effects of changes done to "past incarnations" of the ring.

In a sense that's just a clearer manifestation of one of the (multiple) major issues with parent caching: if parents can "learn" information or have settings that affect their behavior, then re-using such a parent later on can have undesirable side effects. This is why e.g. I am reluctant to have caching for GAP-based groups in OSCAR (because GAP groups strongly rely on accumulating attributes and data).

To me an acceptable solution would be to just entirely disable caching for universal polynomial rings (though for backwards compatibility the cached argument would have to be retained).

However, I don't understand why caching is such a big issue for resolving issue #1979 resp. for your PR #1993. I guess I'll have to take a look there and discuss with you to either understand that, or convince you we don't have an issue (to be seen which it'll be ;-) )