Speed up test/operator.jl, fix bug on Julia 1.0 with Base.warn_one. #1443
Conversation
src/JuMP.jl
Outdated
| @@ -702,15 +712,15 @@ a warning is generated once. | |||
| function operator_warn(::AbstractModel) end | |||
| function operator_warn(model::Model) | |||
| model.operator_counter += 1 | |||
| if model.operator_counter > 20000 | |||
| Base.warn_once( | |||
| if model.operator_counter > OPERATOR_WARN_LIMIT | |||
There was a problem hiding this comment.
This introduces a type inference issue. Maybe OPERATOR_WARN_LIMIT::Int here and function set_operator_warn_limit(new_limit::Int) above?
There was a problem hiding this comment.
Is it because OPERATOR_WARN_LIMIT is a non-const module level global?
It only works on JuMP.Model-s anyway - could make a Model field?
There was a problem hiding this comment.
Would have the benefit of making clear the global-ish behaviour that this has already.
There was a problem hiding this comment.
Is it because OPERATOR_WARN_LIMIT is a non-const module level global?
Yep.
could make a Model field?
Seems a bit iffy to add a field to Model just to speed up one test. If the goal is to get test coverage, we might as (1) do some additions and check that operator_counter is nonzero, and (2) call operator_warn directly 20,000 times and make sure it prints the warning.
There was a problem hiding this comment.
In fact, later in this file (test/operators.jl), something similar to that happens!
I'm going to integrate the princple of this change into that, and drop the global.
This comment has been minimized.
This comment has been minimized.
Operator warning used Base.warn_once, which doesn't exist on Julia 1.0. Test was unnecessarily expensive to basically just check one function wasn't called too many times Comment out all other includes in test/runtests.jl, and in test/operator.jl comment out extension test. ``` Before real 2m6.533s user 2m5.312s sys 0m1.349s After real 1m55.578s user 1m54.548s sys 0m1.176s ```
|
Simplified the change a lot, ready for review (as long as tests pass on CI!) |
|
Updated |
Operator warning used Base.warn_once, which doesn't exist on Julia 1.0.
Test was unnecessarily expensive to basically just check one function wasn't
called too many times
Comment out all other includes in test/runtests.jl, and in test/operator.jl
comment out extension test. Saving is probably 20 seconds in full test.