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 for Julia v1.9 by adding check_belongs_to_model #316

Merged
merged 4 commits into from
May 25, 2023

Conversation

odow
Copy link
Contributor

@odow odow commented May 11, 2023

Closes #315

I've very confused as to why this is needed...

@pulsipher
Copy link
Collaborator

We'll see what happens with the tests. Super weird that this is needed and even weirder if it works. Maybe some bug in the Julia compiler?

@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Merging #316 (b7da6e3) into master (b1c835d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #316   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          36       36           
  Lines        7099     7103    +4     
=======================================
+ Hits         7083     7087    +4     
  Misses         16       16           
Impacted Files Coverage Δ
src/infinite_variables.jl 99.54% <100.00%> (+<0.01%) ⬆️

@odow
Copy link
Contributor Author

odow commented May 11, 2023

Maybe some bug in the Julia compiler?

Yeah. It feels like an issue with precompilation. Here's my current minimal reproducer:

(base) oscar@Oscars-MBP /tmp % julia +1.9 --project=ifo
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.0 (2023-05-07)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

(ifo) pkg> st
Status `/private/tmp/ifo/Project.toml`
  [20393b10] InfiniteOpt v0.5.7

julia> versioninfo()
Julia Version 1.9.0
Commit 8e630552924 (2023-05-07 11:25 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin22.4.0)
  CPU: 8 × Intel(R) Core(TM) i5-8259U CPU @ 2.30GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-14.0.6 (ORCJIT, skylake)
  Threads: 1 on 8 virtual cores

julia> using InfiniteOpt

julia> function my_add_variable(model, v)
           y = InfiniteOpt.dispatch_variable_ref(v.infinite_variable_ref)
           JuMP.check_belongs_to_model(y, model)
           return
       end
my_add_variable (generic function with 1 method)

julia> m1 = InfiniteOpt.InfiniteModel();

julia> InfiniteOpt.@infinite_parameter(m1, p in [0, 1]);

julia> InfiniteOpt.@variable(m1, x, Infinite(p));

julia> v = JuMP.build_variable(
           error, 
           JuMP.VariableInfo(false, 0.0, false, 0.0, false, 0.0, false, 0.0, false, false), 
           InfiniteOpt.Point(x, 0.5),
       );

julia> m2 = InfiniteOpt.InfiniteModel();

julia> my_add_variable(m2, v)  # When called via a function, does not error

julia> y = dispatch_variable_ref(v.infinite_variable_ref)
x(p)

julia> JuMP.check_belongs_to_model(y, m2)  # When called via REPL, does error
ERROR: VariableNotOwned{InfiniteVariableRef}(x(p))
Stacktrace:
 [1] check_belongs_to_model(v::InfiniteVariableRef, model::InfiniteModel)
   @ JuMP ~/.julia/packages/JuMP/AKvOr/src/variables.jl:282
 [2] top-level scope
   @ REPL[12]:1

Am I doing anything stupid? If not, I'll open an issue upstream.

@pulsipher
Copy link
Collaborator

Am I doing anything stupid? If not, I'll open an issue upstream.

I think I can simplify this, give me a few minutes to check. Installing Julia 1.9....

@pulsipher
Copy link
Collaborator

pulsipher commented May 11, 2023

@odow I didn't succeed in getting a simpler case which confuses me even more. So yes, your MWE should be good to go to raise an issue.

@pulsipher
Copy link
Collaborator

With the latest change, strangely using DispatchVariableRef which is a super type of InfiniteVariableRef instead of InfiniteVariableRef, causes the bug to reappear again...

@pulsipher
Copy link
Collaborator

Requires #314 for tests to pass.

@pulsipher
Copy link
Collaborator

With the latest change, strangely using DispatchVariableRef which is a super type of InfiniteVariableRef instead of InfiniteVariableRef, causes the bug to reappear again...

Changing it back to InfiniteVariableRef resolves the issue, still strange that this is needed and dispatching doesn't seem to be working properly.

@pulsipher pulsipher merged commit ba79f48 into infiniteopt:master May 25, 2023
@odow odow deleted the patch-1 branch May 25, 2023 19:49
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.

Test failure on Julia 1.9
2 participants