Simplify promote_operation_fallback
#335
Conversation
AayushSabharwal
force-pushed
the
as+bl/simplify_promote_type_fallback
branch
from
68975a9 to
f99b96a
Compare
|
The JuMP tests fail because MA.promote_operation(MA.add_mul, QuadExpr, Int, GenericQuadExpr{ComplexF64, VariableRef})returns GenericQuadExpr{CoefType, VariableRef} where CoefTypeinstead of GenericQuadExpr{ComplexF64, VariableRef}as it did before |
|
The answer should be |
|
We can fix it with a PR to JuMP. At the moment, it's a performance issue with JuMP since the corresponding |
|
Do you mean this method? julia> using JuMP
julia> import JuMP._MA as MA
julia> begin
model = Model()
@variable(model, x)
ret = zero(QuadExpr)
f = (1.0 + 2.0im) * x^2
MA.add_mul(ret, 2, f)
end
(2 + 4im) x²What do you want to fix in JuMP? |
|
In JuMP, |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #335 +/- ##
==========================================
+ Coverage 90.50% 90.55% +0.05%
==========================================
Files 22 22
Lines 2253 2265 +12
==========================================
+ Hits 2039 2051 +12
Misses 214 214 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Should be fixed by jump-dev/JuMP.jl#4057 |
AayushSabharwal
force-pushed
the
as+bl/simplify_promote_type_fallback
branch
from
f99b96a to
2e089c2
Compare
AayushSabharwal
force-pushed
the
as+bl/simplify_promote_type_fallback
branch
3 times, most recently
from
61de1dd to
8d48c21
Compare
| if isconcretetype(S) && isconcretetype(T) | ||
| return typeof(op(_instantiate_zero(S), _instantiate_zero(T))) | ||
| else | ||
| return promote_type(S, T) |
| if isconcretetype(S) && isconcretetype(T) | ||
| return typeof(op(_instantiate_zero(S), _instantiate_oneunit(T))) | ||
| else | ||
| return promote_type(S, T) |
| ::Union{typeof(real),typeof(imag)}, | ||
| ::Type{Complex{T}}, | ||
| ) where {T} | ||
| return T |
src/interface.jl
Outdated
| ::Type{S}, | ||
| ::Type{LinearAlgebra.UniformScaling{T}}, | ||
| ) where {S,T} | ||
| return promote_operation(op, S, T) |
There was a problem hiding this comment.
How is this related to this PR ?
There was a problem hiding this comment.
promote_type(Number, LinearAlgebra.UniformScaling{Bool}) ends up as Any, which is incorrect.
There was a problem hiding this comment.
Actually, this is avoidable
AayushSabharwal
force-pushed
the
as+bl/simplify_promote_type_fallback
branch
2 times, most recently
from
afc95c1 to
aaa3e5c
Compare
| end | ||
|
|
||
| function promote_operation_fallback( | ||
| ::Union{typeof(real),typeof(imag)}, |
There was a problem hiding this comment.
This should be promote_operation, it's not a fallback
Co-authored-by: Benoît Legat <benoit.legat@gmail.com>
AayushSabharwal
force-pushed
the
as+bl/simplify_promote_type_fallback
branch
from
aaa3e5c to
777be85
Compare
Continuation of #334