-
Notifications
You must be signed in to change notification settings - Fork 94
Fix various JET errors #2276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix various JET errors #2276
Conversation
MOI.VectorQuadraticFunction{T}, | ||
MOI.VectorNonlinearFunction, | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. This breaks JuMP:
https://github.com/jump-dev/JuMP.jl/blob/1ee5faee4335386435e3211b8758fba13799234e/test/test_objective.jl#L168-L180
I'm not sure whether we intended this:
julia> model = GenericModel{Float32}();
julia> @variable(model, x)
x
julia> @objective(model, Min, 2 * x + 1)
2 x + 1
julia> f = objective_function(model, GenericAffExpr{Float64,VariableRef})
2 x + 1
julia> typeof(f)
GenericAffExpr{Float64, GenericVariableRef{Float32}}
julia> g = objective_function(model)
2 x + 1
julia> typeof(g)
GenericAffExpr{Float32, GenericVariableRef{Float32}}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I think we meant to use T
instead of Float64
in the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I guess we just leave this as-is for now though.
return x | ||
end | ||
|
||
function MOI.add_variables(model::AbstractModel, n::Integer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did that one go ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the same as the default fallback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, it's in Utilities/model.jl
. We indeed used to not have that default fallback
Part of #2268
Now down to
═════ 70 possible errors found ═════
https://github.com/jump-dev/MathOptInterface.jl/actions/runs/6242312138