-
Notifications
You must be signed in to change notification settings - Fork 81
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
WIP: Add a silent option to environment constructor #456
Conversation
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.
CI failed because you pushed to your simonbowly
fork. If you want to run the CI, you can push to the jump-dev
remote:
git add remote jump-dev https://github.com/jump-dev/Gurobi.jl.git
git push -u jump-dev silent-env
src/MOI_wrapper/MOI_wrapper.jl
Outdated
@@ -97,10 +97,15 @@ mutable struct Env | |||
finalize_called::Bool | |||
attached_models::Int | |||
|
|||
function Env() | |||
function Env(::MOI.Silent, flag::Bool) |
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.
Why not just Env(; output_flag::Int = 1)
?
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.
I figured you might want alignment with ::Silent for parameter settings, but sure, this makes more sense in a constructor.
Ah, I may have found the issue: Gurobi.jl/src/MOI_wrapper/MOI_wrapper.jl Lines 382 to 388 in 1648d6e
Looks like we set silent after we change other parameters 😥 |
Ah, nice one! And that's called in every constructor? |
If you create a new |
I still get a couple of outputs when running the tests with that change. Are there a few Silent=False settings hanging about?
|
In MOI.empty(), set outputflag=0 before any other parameter changes are made.
Set output_flag=0 for all environments used in the tests
11e5ca5
to
2b9d1ff
Compare
CI test run for just the parameter ordering change is here - https://github.com/simonbowly/Gurobi.jl/runs/5240766098?check_suite_focus=true This cleans up everything except the licensing messages, second commit is needed to do that. |
Can you push to the |
Possible fix for the stray output you are seeing with 9.5 as noted in #437