Fix diagm_container for types without zero #35743
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For some types,
T
,zero(T) isa T
does not hold. This is the case for instance forT = JuMP.VariableRef
for whichzero(T) isa AffExpr
. When the users calldiagm
with vectors ofVariableRef
, we should have a matrix ofAffExpr
but instead an error is thrownbecause
diagm_container
callszeros(JuMP.VariableRef, ...)
.I think it is fair for
zeros(JuMP.VariableRef, ...)
to error and it would not be consistent for it to return a matrix ofJuMP.AffExpr
so it seems the issue is thatdiagm_container
callszeros(JuMP.VariableRef, ...)
hence the fix in this PR.Note that if you try it with the latest version of JuMP, it currently works as we redefine
diagm_container
just for this case:https://github.com/JuliaOpt/MutableArithmetics.jl/blob/407a8fdfb5ef1810f3785a88e490291d37b9fb03/src/dispatch.jl#L27-L31
but we'd rather have it fixed upstream as other types might benefit from it (e.g. polynomial variables from MultivariatePolynomials).
See jump-dev/MutableArithmetics.jl#47 for more context.