Extend // to create total_ring_of_fractions if necessary

[JohnAAbbott]

First prototype.
Quick test:

julia> ZZ100,_ = residue_ring(ZZ,100);

julia> ten = ZZ100(10);

julia> 1//ten
ERROR: Division by a zero-divisor

julia> 1//(ten+1)
1//11

[JohnAAbbott]

This is proving a little trickier than anticipated: some tests create, e.g., residue_ring(ZZ,5) and then use // on its elements which creates a result in a FractionField. In the test cases this is actually correct because the starting ring has no non-zero zero-divisors. The revised code sees a starting ring for which is_integral_type gives false, so the result is a total_ring_of_fractions. The consequence is that some functions (e.g. evaluate) which accept a FracFieldElem no longer match the given arguments (of type TotFrac). Should I just chase through and extend these other functions to accept FracFieldElem or TotFrac?? @thofma

[thofma]

This is proving a little trickier than anticipated: some tests create, e.g., residue_ring(ZZ,5) and then use // on its elements which creates a result in a FractionField. In the test cases this is actually correct because the starting ring has no non-zero zero-divisors. The revised code sees a starting ring for which is_integral_type gives false, so the result is a total_ring_of_fractions. The consequence is that some functions (e.g. evaluate) which accept a FracFieldElem no longer match the given arguments (of type TotFrac). Should I just chase through and extend these other functions to accept FracFieldElem or TotFrac?? @thofma

I think that is the way to go. I would probably just do the minimal number of changes to make the tests pass. Should hopefully not be too many.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.13%. Comparing base (33141c2) to head (7cd46d0).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2410      +/-   ##
==========================================
- Coverage   88.13%   88.13%   -0.01%     
==========================================
  Files         130      130              
  Lines       32948    32970      +22     
==========================================
+ Hits        29038    29057      +19     
- Misses       3910     3913       +3     

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

[JohnAAbbott]

It's looking better now. What do you think @thofma ?

[JohnAAbbott]

I believe this is now ready. @thofma?

[thofma]

Looks good, thanks. As it turns out, Oscar has the type hardcoded, which is breaking Oscar:

https://github.com/oscar-system/Oscar.jl/blob/master/src/Rings/mpoly-localization_types.jl#L367

struct MPolyLocRingElem{
    BaseRingType, 
    BaseRingElemType,
    RingType,
    RingElemType, 
    MultSetType
  } <: AbsLocalizedRingElem{
    RingType,
    RingElemType, 
    MultSetType
  } 

  W::MPolyLocRing{BaseRingType, BaseRingElemType, RingType, RingElemType, MultSetType}
  frac::AbstractAlgebra.Generic.FracFieldElem{RingElemType}

So the type will hold elements of a fraction field of $R[x_1,...,x_n]$. This was always FracFieldElem before. But with the changes here, for R, = quo(ZZ, 5) this now constructs the total fraction field:

https://github.com/oscar-system/Oscar.jl/blob/master/test/AlgebraicGeometry/Schemes/AffineSchemes.jl#L277-L282

One simple fix would be to use GF(5) in the tests instead, so that corresponding multivariate polynomial rings have a domain type. Would that work for you @HechtiDerLachs?