infiniteopt / InfiniteOpt.jl

An intuitive modeling interface for infinite-dimensional optimization problems.
https://infiniteopt.github.io/InfiniteOpt.jl/stable
MIT License
251 stars 17 forks source link

Fix for Julia v1.9 by adding check_belongs_to_model #316

Closed odow closed 1 year ago

odow commented 1 year ago

Closes #315

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

pulsipher commented 1 year ago

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[bot] commented 1 year ago

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%) :arrow_up:
odow commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

pulsipher commented 1 year ago

Requires #314 for tests to pass.

pulsipher commented 1 year ago

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.