JuliaIntervals / IntervalConstraintProgramming.jl

Calculate rigorously the feasible region for a set of real-valued inequalities with Julia
Other
63 stars 16 forks source link

Make ModelingToolkit optional - alternative #210

Closed schillic closed 5 months ago

schillic commented 6 months ago

This is an alternative to #209 where the last commit was replaced by two new commits

  1. Make the ModelingToolkit tests optional instead, as suggested by @lucaferranti.
  2. Add CI runs on older Julia versions so that the ModelingToolkit code is still tested.
codecov-commenter commented 6 months ago

Codecov Report

Attention: Patch coverage is 74.69880% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 75.00%. Comparing base (4ddec09) to head (a8f979c).

Files Patch % Lines
src/separator.jl 59.45% 15 Missing :warning:
src/contractor.jl 78.57% 6 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #210 +/- ## ========================================== + Coverage 74.86% 75.00% +0.13% ========================================== Files 9 10 +1 Lines 541 556 +15 ========================================== + Hits 405 417 +12 - Misses 136 139 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dpsanders commented 5 months ago

Thanks! I'm happy to merge either this or #209 . Which do you prefer?

dpsanders commented 5 months ago

I see, they're basically the same but this still tests older versions. This seems like the best option then.

dpsanders commented 5 months ago

I'm really not sure there's much point in testing on Julia 1.3 though.

dpsanders commented 5 months ago

Just to confirm: This makes it possible to use the package with Julia 1.10? Which version of ModelingToolkit does that use?

dpsanders commented 5 months ago

Doing using ModelingToolkit with this branch (in a temporary project) gives

┌ Warning: Error requiring `ModelingToolkit` from `IntervalConstraintProgramming`
│   exception =
│    LoadError: UndefVarError: `Constant` not defined
│    Stacktrace:
│      [1] include(mod::Module, _path::String)
│        @ Base ./Base.jl:495
│      [2] include(x::String)
│        @ IntervalConstraintProgramming ~/Dropbox/packages/IntervalConstraintProgramming/src/IntervalConstraintProgramming.jl:3
│      [3] top-level scope
│        @ ~/.julia/packages/Requires/Z8rfN/src/Requires.jl:40
│      [4] eval
│        @ ./boot.jl:385 [inlined]
│      [5] eval
│        @ ~/Dropbox/packages/IntervalConstraintProgramming/src/IntervalConstraintProgramming.jl:3 [inlined]
│      [6] (::IntervalConstraintProgramming.var"#23#26")()
│        @ IntervalConstraintProgramming ~/.julia/packages/Requires/Z8rfN/src/require.jl:101
│      [7] macro expansion
│        @ timing.jl:395 [inlined]
│      [8] err(f::Any, listener::Module, modname::String, file::String, line::Any)
│        @ Requires ~/.julia/packages/Requires/Z8rfN/src/require.jl:47
│      [9] (::IntervalConstraintProgramming.var"#22#25")()
│        @ IntervalConstraintProgramming ~/.julia/packages/Requires/Z8rfN/src/require.jl:100
│     [10] withpath(f::Any, path::String)
│        @ Requires ~/.julia/packages/Requires/Z8rfN/src/require.jl:37
│     [11] (::IntervalConstraintProgramming.var"#21#24")()
│        @ IntervalConstraintProgramming ~/.julia/packages/Requires/Z8rfN/src/require.jl:99
│     [12] #invokelatest#2
│        @ ./essentials.jl:892 [inlined]
│     [13] invokelatest
│        @ ./essentials.jl:889 [inlined]
│     [14] foreach(f::typeof(invokelatest), itr::Vector{Function})
│        @ Base ./abstractarray.jl:3097
│     [15] loadpkg(pkg::Base.PkgId)
│        @ Requires ~/.julia/packages/Requires/Z8rfN/src/require.jl:27
│     [16] #invokelatest#2
│        @ ./essentials.jl:892 [inlined]
│     [17] invokelatest
│        @ ./essentials.jl:889 [inlined]
│     [18] run_package_callbacks(modkey::Base.PkgId)
│        @ Base ./loading.jl:1169
│     [19] __require_prelocked(uuidkey::Base.PkgId, env::String)
│        @ Base ./loading.jl:1819
│     [20] #invoke_in_world#3
│        @ ./essentials.jl:926 [inlined]
│     [21] invoke_in_world
│        @ ./essentials.jl:923 [inlined]
│     [22] _require_prelocked(uuidkey::Base.PkgId, env::String)
│        @ Base ./loading.jl:1803
│     [23] macro expansion
│        @ ./loading.jl:1790 [inlined]
│     [24] macro expansion
│        @ ./lock.jl:267 [inlined]
│     [25] __require(into::Module, mod::Symbol)
│        @ Base ./loading.jl:1753
│     [26] #invoke_in_world#3
│        @ ./essentials.jl:926 [inlined]
│     [27] invoke_in_world
│        @ ./essentials.jl:923 [inlined]
│     [28] require(into::Module, mod::Symbol)
│        @ Base ./loading.jl:1746
│     [29] eval
│        @ ./boot.jl:385 [inlined]
│     [30] eval_user_input(ast::Any, backend::REPL.REPLBackend, mod::Module)
│        @ REPL ~/.julia/juliaup/julia-1.10.2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:150
│     [31] repl_backend_loop(backend::REPL.REPLBackend, get_module::Function)
│        @ REPL ~/.julia/juliaup/julia-1.10.2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:246
│     [32] start_repl_backend(backend::REPL.REPLBackend, consumer::Any; get_module::Function)
│        @ REPL ~/.julia/juliaup/julia-1.10.2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:231
│     [33] run_repl(repl::REPL.AbstractREPL, consumer::Any; backend_on_current_task::Bool, backend::Any)
│        @ REPL ~/.julia/juliaup/julia-1.10.2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:389
│     [34] run_repl(repl::REPL.AbstractREPL, consumer::Any)
│        @ REPL ~/.julia/juliaup/julia-1.10.2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/REPL/src/REPL.jl:375
│     [35] (::Base.var"#1013#1015"{Bool, Bool, Bool})(REPL::Module)
│        @ Base ./client.jl:432
│     [36] #invokelatest#2
│        @ ./essentials.jl:892 [inlined]
│     [37] invokelatest
│        @ ./essentials.jl:889 [inlined]
│     [38] run_main_repl(interactive::Bool, quiet::Bool, banner::Bool, history_file::Bool, color_set::Bool)
│        @ Base ./client.jl:416
│     [39] exec_options(opts::Base.JLOptions)
│        @ Base ./client.jl:333
│     [40] _start()
│        @ Base ./client.jl:552
│    in expression starting at /Users/dsanders/Dropbox/packages/IntervalConstraintProgramming/src/init_ModelingToolkit.jl:1
└ @ Requires ~/.julia/packages/Requires/Z8rfN/src/require.jl:51
dpsanders commented 5 months ago

The @constraint functionality does work without doing using ModelingToolkit, though. Is this the desired effect of this PR?

dpsanders commented 5 months ago

I think ModelingToolkit no longer has Constant?

Which version of ModelingToolkit is supposed to be used?

dpsanders commented 5 months ago

Is the user supposed to manually install a particular version of ModelingToolkit? Is there a way to enforce this?

schillic commented 5 months ago

Thanks for taking a look. That's a lot of questions - let's see :)

Which do you prefer? ... I see, they're basically the same but this still tests older versions. This seems like the best option then.

Yes, I guess this one is slightly better.

This makes it possible to use the package with Julia 1.10?

Exactly, that is the point.

Which version of ModelingToolkit does that use?

Whatever Pkg allows and you tell it to use.

Doing using ModelingToolkit with this branch (in a temporary project) gives ... I think ModelingToolkit no longer has Constant?

Seems so, yes. But you could install the older version 3 and get the same as before. Just not in Julia v1.10.

The @constraint functionality does work without doing using ModelingToolkit, though. Is this the desired effect of this PR?

Yes, exactly, because it does not require ModelingToolkit. I want to be able to use the package in Julia v1.10, which currently is only possible without ModelingToolkit.

Is the user supposed to manually install a particular version of ModelingToolkit?

Yes, if you want to use it. That is the whole point of making it optional. You are not bound to install it anymore, which is the only way to make the package work with v1.10.

Is there a way to enforce this?

No. Either it is a required dependency (old behavior) or optional without any control on the version.

Well, technically one could check for the package version when it is loaded and then print a warning when the version is not desired. I can add such a check if you want.

I am also happy to add a note about this somewhere. If desired, where should I put it?


So to be explicit about this:

master:

Julia version old new
works? yes no
ModelingToolkit? required n/a

This branch:

Julia version old new
works? yes yes
ModelingToolkit? optional crashes

I have not checked whether there is a version of ModelingToolkit that still works in v1.10. But I would argue that then it would be better to just support a newer version directly (and actually then one should support Symbolics).

dpsanders commented 5 months ago

Thanks for your replies!

Let's go ahead and merge this. If you could add a note in the README about ModelingToolkit that would be great.

I'm not sure when I'll be able to take up #204 again so let's at least have as much of the functionality available as possible. Especially since I tried an example and remembered that it's pretty neat ;)

dpsanders commented 5 months ago

Many thanks @schillic !

dpsanders commented 5 months ago

Hmm OK now that I merged it I see that the version and compat bounds in Project.toml don't seem right. Are you able to fix those up in a new PR?

schillic commented 5 months ago

What exactly is wrong?

dpsanders commented 5 months ago

The allowed versions of Julia and the other compat entries I think?

dpsanders commented 5 months ago

And the new version of this package (though I can fix that).

schillic commented 5 months ago

After merging, this is Project.toml's content:

name = "IntervalConstraintProgramming"
uuid = "138f1668-1576-5ad7-91b9-7425abbf3153"
version = "0.12.4"

[deps]
IntervalArithmetic = "d1acc4aa-44c8-5952-acd4-ba5d80a2a253"
IntervalContractors = "15111844-de3b-5229-b4ba-526f2f385dc9"
MacroTools = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09"
Requires = "ae029012-a4dd-5104-9daa-d747884805df"

[compat]
IntervalArithmetic = "0.16, 0.17, 0.18, 0.19, 0.20"
IntervalContractors = "0.4"
MacroTools = "0.4, 0.5"
Requires = "0.5, 1"
julia = "1.3"

I think this is fine. Julia 1.3 stands for 1.x where x >= 3. IntervalArithmetic v0.21 is not supported, IntervalContractors v0.4 is the latest minor version, MacroTools v0.5 is the latest minor version, and Requires v1 is the latest major version.

Yes, please make a new release :)

schillic commented 5 months ago

I also opened a PR with an update for the README (#211).

dpsanders commented 5 months ago

OK great, I started the release process. Many thanks!