iszero

[blegat]

Not high priority but this won't work if the SparseCoefficients is not canonicalized:

StarAlgebras.jl/src/coefficients.jl

Line 37 in ba3697f

Base.iszero(ac::AbstractCoefficients) = isempty(keys(ac))

This doesn't work for tuples:

StarAlgebras.jl/src/types.jl

Line 92 in ba3697f

Base.iszero(a::AlgebraElement) = iszero(coeffs(a))

[kalmarek]

Yes, we need to fix our semantics and develop a consistent mindset for dealing with canonicalization.

1. Is is a temporary state within unsafe_ functions?
2. If yes, do we bring the coefficients to canonical form upon construction, or should we trust the user?
3. does canonical always perform its task, or is is a no-op (and hence we can call it freely in our code) for already canonicalized coeffs?

especially the latter will experience problems with thread-safety, but maybe its not our goal?

[blegat]

You can only bring it to a non-canonical state if we call unsafe_ function of UnsafeAddMul then you are responsible to call canonicalize!. I don't think it's a no-op then, we need canonicalize! to do its thing right. If the user calls it, it means the user know that he did unsafe things so he's calling it explicitly. If the user never calls unsafe methods then he should never have to canonicalize. If he does, then it's not a no-op and there is a cost, so he should just not do it ^^

[kalmarek]

if so, then I don't understand where is the issue:

• Base.iszero is a user-facing function
• user will not see non-canonicalized elements using only user-facing functions

It seems that Base.iszero is correctly implemented then?
Maybe the only thing that is missing is that we canonicalize at creation?

[blegat]

Yes, I guess that's all that's missing then, and stating this rule in the docstring and then we are good I think