Enabled poly ring caching in charpoly/minpoly (when no poly ring passed in)

[JohnAAbbott]

After discussion in triage meeting today: now characteristic_polynomial(M) == characteristic_polynomial(M) works, and similarly for minpoly(M). The automatically created polynomial ring is cached. I also commented why caching is enabled.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.34%. Comparing base (956a964) to head (ef1c947).
Report is 35 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1976   +/-   ##
=======================================
  Coverage   88.34%   88.34%           
=======================================
  Files         124      124           
  Lines       31404    31404           
=======================================
  Hits        27743    27743           
  Misses       3661     3661           

☔ 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]

I am sympathetic to this change, but it would mean fixing all charpoly/characteristic_polynomial calls with a single argument in Nemo/Hecke/Oscar (functions are not allowed to create cached objects).

It becomes also be a bit unergonomic to create characteristic polynomials without caching the parent of the polynomial. I wonder whether something like charpoly(M; cached::Bool = true) would make sense. It sounds a bit ambiguous though (is it caching the characteristic polynomial?!).

Happy to hear what others think.

[fingolfin]

Some "thoughts" from discussion during triage (will add some more details later)

# used by "end users"
charpoly(M)


charpoly(M; cached::Bool=true)
charpoly(M; cached_parent::Bool=true)

# or just require library authors to write this?  
charpoly(poly_ring(base_ring(M)), M)



charpoly(ring, M)


charpoly(M; parent::Union{Nothing,PolyRing}=nothing, cached_parent::Bool=true)


# another option: pass in the "variable" / transcendetal element,
# and return the poly evaluated at x (x could be a polynomial
# variable but more generally any ring element)
charpoly(M, x::RingElement)

Regarding the question of how much code we need to adjust library code calling this: we should first investigate this some more. @thofma suggested we could e.g. add the cached=true kwarg but then assert that people actually pass in false just to help find places that need to be adjusted.

[lgoettgens]

Also see oscar-system/Oscar.jl#4520 and oscar-system/Oscar.jl#4518 for previous discussion about this

[fingolfin]

I think the rough plan from last time was to add a cached argument but for now let it at false, and then go through code in our packages to adjust it and/or to see if there are blockers were adjusting will be difficult.

Now "someone" needs to do that.