Revise check_parent for ResElem and ResFieldElem

[fingolfin]

Also remove check_parent_type.

Previously, we allowed binary operations involving two ResElem instances of different type, with the type of the result depending on the argument order. For example, this (requires Nemocas/Nemo.jl#1819):

julia> using Nemo

julia> R = quo(ZZ, 2)[1]
Integers modulo 2

julia> S = Nemo.AbstractAlgebra.EuclideanRingResidueRing{UInt}(UInt(2))
Residue ring of integers modulo 2

julia> typeof(R)
zzModRing

julia> typeof(S)
EuclideanRingResidueRing{UInt64}

julia> S(1) + R(1)
0

julia> typeof(S(1) + R(1))
EuclideanRingResidueRingElem{UInt64}

julia> typeof(R(1) + S(1))
zzModRingElem

But this seems questionable at best. I think we are better of forbidding this -- which this PR effectively does.

However, I am sure people had a reason to add the code I am modifying here in the first place. So this definitely shouldn't be merged before hearing from at least @fieker and @thofma who probably know more about the background of this than I do.

Like Nemocas/Nemo.jl#1819 this also fixes the "introduction" notebook of the OSCAR book.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.39%. Comparing base (72a7117) to head (c5e121a).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1741      +/-   ##
==========================================
- Coverage   87.39%   87.39%   -0.01%     
==========================================
  Files         117      117              
  Lines       29929    29927       -2     
==========================================
- Hits        26157    26155       -2     
  Misses       3772     3772              

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

[joschmitt]

The notebook works (errors in the expected way) with this pull request and then the Nemo one seems not necessary?

[fingolfin]

I think the Nemo one still makes sense independently of this one here, which is why I submitted both.

Also, for this one it is not quite clear to me why we treat residue rings different from other kinds of rings, where we insist on equality for parents. Why do we make an exception here?

[thofma]

I agree that it should just be removed.

[thofma]

P.S.: What is the difference of the proposed change and parent(a) === parent(b)?

[fingolfin]

The difference is that if you now created two different (non-cached) but mathematically equal residue rings we still allow adding their elements. Unlike basically any other kind of ring, I think?

If you know ask "but why do we want that?" then all I can say "I have no idea" -- personally I'd be happy to just require parent(a) === parent(b) (by just removing the two check_parent methods in this PR and letting the default method take over). But I don't know if this would break any code...

BTW I could see an argument for supporting this for parents that have same type but different modulus, if one modulus divides the other (but not the other way around) -- then there is a somewhat natural interpretation (the result is defined over the ring with the "smaller" modulus). Personally I am not a fan, I'd prefer explicit coercion in this case, too, but I could understand if someone would like this because they need it all the time shrug

[thofma]

No clue why we did it back then. Ideally it is just parent(a) === parent(b), but I agree that we should probably play it safe for now.