Wikunia / ConstraintSolver.jl

ConstraintSolver in Julia: Blog posts ->
https://opensourc.es/blog/constraint-solver-1
MIT License
136 stars 13 forks source link

Constraint programming extensions #270

Closed Wikunia closed 2 years ago

Wikunia commented 2 years ago

Using ConstraintProgrammingExtensions.jl for

Ping @dourouc05 as you might be interested :wink:

codecov[bot] commented 2 years ago

Codecov Report

Merging #270 (32e2dcb) into master (63af545) will decrease coverage by 0.05%. The diff coverage is 93.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #270      +/-   ##
==========================================
- Coverage   97.60%   97.55%   -0.06%     
==========================================
  Files          53       53              
  Lines        4469     4458      -11     
==========================================
- Hits         4362     4349      -13     
- Misses        107      109       +2     
Impacted Files Coverage Δ
src/MOI_wrapper/Bridges/util.jl 100.00% <ø> (ø)
src/constraints/all_equal.jl 95.87% <ø> (ø)
src/constraints/not_equal.jl 100.00% <ø> (ø)
src/MOI_wrapper/Bridges/strictly_greater_than.jl 75.00% <60.00%> (ø)
src/ConstraintSolver.jl 98.57% <100.00%> (-0.57%) :arrow_down:
src/MOI_wrapper/constraints.jl 99.59% <100.00%> (-0.01%) :arrow_down:
src/constraints/all_different.jl 100.00% <100.00%> (ø)
src/constraints/linear_constraints.jl 97.32% <100.00%> (ø)
src/lp_model.jl 90.62% <100.00%> (ø)
src/simplify.jl 98.78% <100.00%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 63af545...32e2dcb. Read the comment docs.

Wikunia commented 2 years ago

Haha I remember I copied Strictly from you. I probably just misread that. Yeah looks the same to me 😆

dourouc05 commented 2 years ago

I thought that this kind of commit would reduce the amount of code in the solver, but you're adding 50 lines with it :/. Do you think anything should be done better in CPE? (While remaining at MOI level, without dependency on JuMP, that is.)

Wikunia commented 2 years ago

The extra lines are probably from the manifest file. Otherwise I only removed lines and added your package

odow commented 2 years ago

Great to see this coming along.

dourouc05 commented 2 years ago

For users, maybe it would be best if you provided @deprecate replacements for building sets? This would print a warning to change the calls, which could make updating their version of your solver easier (and you could remove these lines when tagging a v0.8, for instance). Normally, the syntax is: @deprecate old new false, without parentheses (I just hope that it works across packages :/).

Wikunia commented 2 years ago

I'm actually wondering whether I should export ConstraintSolverExtensions.AllDifferent and others, such that the user can use AllDifferent instead of needing to have using ConstraintSolverExtensions as well. What do you think in general about how solvers should handle this @dourouc05 ?

Oh I forgot the user will use my things anyway as yours need the dim and I have the layer around it.

dourouc05 commented 2 years ago

Based on what MOI solvers have, they do not export anything: rather, every set is exported by JuMP if need be (and the symbol MOI is exported by JuMP too, so that you have direct access for sets that do not have a shortcut in JuMP). In my opinion, it's the job of JuCP (or whatever it ends up being called) to do that; in the meantime, it's probably OK that solvers do it too.

dourouc05 commented 2 years ago

Indeed, I don't have yet bridges to reverse inequalities. Do you want to make a PR with https://github.com/Wikunia/ConstraintSolver.jl/blob/master/src/MOI_wrapper/Bridges/strictly_greater_than.jl or can I copy that code?

Wikunia commented 2 years ago

Feel free to copy the code :wink:

dourouc05 commented 2 years ago

Yay :D! I just added a link to this solver from CPE's README.

dourouc05 commented 2 years ago

If you want, you can replace your bridge by the new one: https://github.com/dourouc05/ConstraintProgrammingExtensions.jl/commit/660a0d6225d5a33f2a977e57ff97e8232cbd0637. I don't think your other bridges make sense in CPE for the moment, as long as we haven't settled on how to represent Boolean constraints.