Skip to content
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 printing in JuMP v1.11.1 #314

Merged
merged 5 commits into from
May 25, 2023
Merged

Conversation

odow
Copy link
Contributor

@odow odow commented May 11, 2023

The upcoming JuMP 1.11.1 tweaks how we do printing. I'm not really sure if this is a "breaking" change or not, because it affects only the tests of InfiniteOpt, and the exact string that is printed isn't part of the API contract.

What's weird is that I couldn't reproduce the failures: jump-dev/JuMP.jl#3350 (comment)

@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Merging #314 (3ad1dc8) into master (a66bbde) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #314   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          36       36           
  Lines        7099     7099           
=======================================
  Hits         7083     7083           
  Misses         16       16           

@odow odow mentioned this pull request May 11, 2023
@pulsipher
Copy link
Collaborator

pulsipher commented May 11, 2023

The remaining errors all stem from JuMP.check_belongs_to_model not throwing an error when expected on Julia 1.9. In particular, the following test fails:

@testset "JuMP.add_variable" begin
# prepare secondary model and infinite variable
m2 = InfiniteModel()
@infinite_parameter(m2, pref3 in [0, 1])
@variable(m2, ivref3, Infinite(pref3))
v = build_variable(error, info, Point(ivref3, 0.5))
# test for invalid variable error
@test_throws VariableNotOwned{InfiniteVariableRef} add_variable(m, v)

which causes the subsequent checks to fail since a variable is unexpected added to the model at first. This test is intended to trigger the JuMP.check_belongs_to_model here:
function JuMP.add_variable(
model::InfiniteModel,
v::PointVariable,
name::String = "";
add_support = true,
update_info = true
)::GeneralVariableRef
ivref = v.infinite_variable_ref
divref = dispatch_variable_ref(ivref)
JuMP.check_belongs_to_model(divref, model)

I am not quite sure why this is happening, but I will have some more time later to investigate.

@odow
Copy link
Contributor Author

odow commented May 11, 2023

See #315.

I have a fix that works locally: #316

@odow
Copy link
Contributor Author

odow commented May 11, 2023

That seems to have fixed the v1.9 issue, now the tests fail as expected because of the printing.

@odow
Copy link
Contributor Author

odow commented May 11, 2023

So I guess the question is: are you okay if we release jump-dev/JuMP.jl#3350?

@pulsipher
Copy link
Collaborator

So I guess the question is: are you okay if we release jump-dev/JuMP.jl#3350?

Go ahead, no worries

@pulsipher
Copy link
Collaborator

Requires #316 for 1.9 tests to pass

@pulsipher
Copy link
Collaborator

Merging now that julia 1.6 and docs pass the testing

@pulsipher pulsipher merged commit b1c835d into infiniteopt:master May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants