Make ModelingToolkit optional - alternative #210

schillic · 2024-03-12T07:18:36Z

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

Make the ModelingToolkit tests optional instead, as suggested by @lucaferranti.
Add CI runs on older Julia versions so that the ModelingToolkit code is still tested.

codecov-commenter · 2024-03-12T07:24:07Z

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 ⚠️
src/contractor.jl	78.57%	6 Missing ⚠️

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

Additional details and impacted files

@@            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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dpsanders · 2024-03-16T18:45:13Z

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

dpsanders · 2024-03-16T18:45:52Z

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

dpsanders · 2024-03-16T18:46:25Z

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

dpsanders · 2024-03-16T18:50:06Z

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

dpsanders · 2024-03-16T20:25:55Z

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 · 2024-03-16T20:28:47Z

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

dpsanders · 2024-03-16T21:21:54Z

I think ModelingToolkit no longer has Constant?

Which version of ModelingToolkit is supposed to be used?

dpsanders · 2024-03-16T21:23:05Z

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

schillic · 2024-03-16T22:27:37Z

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 · 2024-03-17T20:11:08Z

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 · 2024-03-17T20:11:26Z

Many thanks @schillic !

dpsanders · 2024-03-17T20:13:24Z

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 · 2024-03-17T20:40:08Z

What exactly is wrong?

dpsanders · 2024-03-17T21:02:03Z

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

dpsanders · 2024-03-17T21:02:32Z

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

schillic · 2024-03-17T21:10:24Z

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 · 2024-03-17T21:54:57Z

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

dpsanders · 2024-03-17T22:18:56Z

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

schillic added 16 commits March 11, 2024 19:22

.gitignore

d995ba6

fix header spaces in NEWS

e764c7f

remove redundant version in Project.toml

a6cb8be

remove REQUIRE files

57818ee

remove .DS_Store

6cd16cf

use isnothing

8099f84

minor formatting

64c0fcc

remove IntervalRootFinding

c1dcb76

remove undefined exports

434506c

prepare for making ModelingToolkit an optional dependency

9d6b302

make ModelingToolkit an optional dependency

c91bf10

use test/Project.toml

39e5fcc

remove unbound arg

c5338ab

add coverage badge

eacda0f

outsource ModelingToolkit tests

c8fc011

add CI runs in older Julia versions

a8f979c

dpsanders merged commit aab7506 into JuliaIntervals:master Mar 17, 2024
20 checks passed

schillic deleted the schillic/alternative branch March 17, 2024 20:40

schillic mentioned this pull request Mar 17, 2024

Make ModelingToolkit optional #209

Closed

schillic mentioned this pull request Apr 1, 2024

Removing refine! from export list #208

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make ModelingToolkit optional - alternative #210

Make ModelingToolkit optional - alternative #210

schillic commented Mar 12, 2024

codecov-commenter commented Mar 12, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

schillic commented Mar 16, 2024

dpsanders commented Mar 17, 2024

dpsanders commented Mar 17, 2024

dpsanders commented Mar 17, 2024

schillic commented Mar 17, 2024

dpsanders commented Mar 17, 2024

dpsanders commented Mar 17, 2024

schillic commented Mar 17, 2024

schillic commented Mar 17, 2024

dpsanders commented Mar 17, 2024

Make ModelingToolkit optional - alternative #210

Make ModelingToolkit optional - alternative #210

Conversation

schillic commented Mar 12, 2024

codecov-commenter commented Mar 12, 2024

Codecov Report

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

dpsanders commented Mar 16, 2024

schillic commented Mar 16, 2024

dpsanders commented Mar 17, 2024

dpsanders commented Mar 17, 2024

dpsanders commented Mar 17, 2024

schillic commented Mar 17, 2024

dpsanders commented Mar 17, 2024

dpsanders commented Mar 17, 2024

schillic commented Mar 17, 2024

schillic commented Mar 17, 2024

dpsanders commented Mar 17, 2024