KlausC / CommutativeRings.jl

CAS, Commutative Rings, Fraction Fields, Quotient Rings, Polynomial Rings, Galois Fields
MIT License
9 stars 3 forks source link

`oneunit(ZZmod{5, Int8}) \ y::Bool` not defined #10

Closed dkarrasch closed 2 years ago

dkarrasch commented 2 years ago

In a nanosoldier run for https://github.com/JuliaLang/julia/pull/43972, I came across this issue: https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/a95ca94_vs_811e534/CommutativeRings.primary.log. That PR tries to make triangular matrices be able to handle unitful eltypes. When solving unittriangular systems, I need to divide by the oneunit of the matrix's eltype, which is where this packages failed in the nanosoldier run. What do you think would be a good solution here. I'm surprised that the zzmod numbers are not subtypes of Number, since then promotion should work out of the box. Instead, it tries to compute oneunit(ZZmod{5, Int8}) \ y::Bool via y / adjoint(oneunit(ZZmod{5, Int8})). One solution could then be to define

Base.adjoint(a::ZZmod) = a

?

KlausC commented 2 years ago

I'm surprised that the zzmod numbers are not subtypes of Number

There is no way to make it a subtype of Number and keep it integrated into the class hierarchy of QuotientRing, which is more general than numbers (for example rational functions of whatever ring elements).

julia> ZZ7 = ZZ/7
ZZmod{7, Int8}

julia> supertype(ZZ7)
QuotientRing{Int8, CommutativeRings.ZZmodClass{7, Int8}}

Instead, it tries to compute oneunit(ZZmod{5, Int8}) \ y::Bool via y / adjoint(oneunit(ZZmod{5, Int8}))

That looks like an error to me. In v"1.8.0-DEV.1504" base/operators.jl:629 we have \(x,y) = adjoint(adjoint(y)/adjoint(x)), which is mathematically correct, but assumes, that x and y may be arguments of adjoint. (So I would expect the missing Base.adjoint(x) = x as a global fallback).

Of course I could define adjoint in my package, but that would not resolve the general case.

KlausC commented 2 years ago

I think best solution is I define \ for Union{Ring,Integer,Rational} as I did already for other common operators.

The fallback definition in base/operators.jl is only correct if adjoint is defined with

(1) (a' * b') == (b * a)'
(2) inv(a') = inv(a)'

which is true for numbers, vectors, and matrices, but not for (user defined) algebraic types in general. For non-commutative groups, I don't think there exists such an adjoint in general (different from inv).

KlausC commented 2 years ago

The issue is resolved in v0.4.0, which has been registered.