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

Glasgow Constraint Solver #202

Merged
merged 38 commits into from
Jul 15, 2024
Merged

Glasgow Constraint Solver #202

merged 38 commits into from
Jul 15, 2024

Conversation

tias
Copy link
Collaborator

@tias tias commented Dec 29, 2022

Matthew's GCS solver interface

Unfinished: multiple constraint tests failing

@tias
Copy link
Collaborator Author

tias commented Dec 30, 2022

Turns out that the transformations were called with different arguments then what they should be.

I updated the docs to make it clearer what they should be.

All the expression tests now pass, so it seems this would be ready to merge...

I do have one request @mmcilree , can we call it 'gcs' instead of 'glassgowconstraintsolver'?
s.SolverLookup.get("glassgowconstraintsolver", m) is really a lot to type... "gcs" would be nicer... just like gcspy is your lib
(we don't support both a shorthand and longhand, just a single one)

@tias
Copy link
Collaborator Author

tias commented Jun 6, 2023

Hi Matthew @mmcilree ,

We've refactored our solver interface and solver making guidelines a bit, based also on this effort, and to make it easier to debug.

The two changes are that there should be a transform() function that does the transformations and returns the list of transformed constraints (very useful for debugging). And then the __add__() function can directly loop over the result of transform and post the constraint one by one (so no more need for a _post_constraint(), that gets merged into the add).

Would you be up for making those changes and testing what the current state is? Would be happy to see it progres...

(and we have a summerschool 2nd week of july, could be nice for participants to also have gcs available)

@Wout4
Copy link
Collaborator

Wout4 commented Oct 11, 2023

I can't seem to install gcspy, did anyone else recently try?
Added it to our automated testing, and altough it says gcspy is succesfully installed the tests still give an error, saying we need to install gcspy

@mmcilree
Copy link
Collaborator

mmcilree commented Oct 11, 2023

Hi @Wout4 the gcspy package is very out of date with respect to the Glasgow constraint solver - lots of the solver API has changed since I wrote the python bindings. It's on my todo list to update it and make sure it all works correctly, I'll try to get to it possibly next week.

Copy link
Collaborator Author

@tias tias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code review, only minor comments

.github/workflows/python-test.yml Outdated Show resolved Hide resolved
cpmpy/solvers/gcs.py Outdated Show resolved Hide resolved
cpmpy/solvers/gcs.py Show resolved Hide resolved
cpmpy/solvers/gcs.py Outdated Show resolved Hide resolved
cpmpy/solvers/gcs.py Outdated Show resolved Hide resolved
cpmpy/solvers/gcs.py Outdated Show resolved Hide resolved
cpmpy/solvers/gcs.py Outdated Show resolved Hide resolved
cpmpy/solvers/utils.py Outdated Show resolved Hide resolved
tests/test_constraints.py Outdated Show resolved Hide resolved
nasty_nesting.py Outdated Show resolved Hide resolved
@tias
Copy link
Collaborator Author

tias commented Jul 11, 2024 via email

@mmcilree mmcilree changed the title Glasgowconstraintsolver Glasgow Constraint Solver Jul 11, 2024
Copy link
Collaborator

@Wout4 Wout4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having some trouble adding an objective to a model:
e.g.

a = boolvar()
m = Model(a == 1)
m.minimize(a).
m.solve('gcs')

This crashes..
Other than that test are successful.

cpmpy/solvers/gcs.py Outdated Show resolved Hide resolved
cpmpy/solvers/gcs.py Show resolved Hide resolved
cpmpy/solvers/gcs.py Outdated Show resolved Hide resolved
@@ -11,6 +11,7 @@
# make sure that `SolverLookup.get(solver)` works
# also add exclusions to the 3 EXCLUDE_* below as needed
SOLVERNAMES = [name for name, solver in SolverLookup.base_solvers() if solver.supported()]
SOLVERNAMES = ['ortools']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should remove this before merging

cpmpy/solvers/gcs.py Show resolved Hide resolved
"alldifferent",
"element",
'table',
'negative_table',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so I guess we could add short_table here ?

elif cpm_expr.name == 'negative_table':
self.gcs.post_negative_table(self.solver_vars(cpm_expr.args[0]), cpm_expr.args[1])
elif isinstance(cpm_expr, GlobalConstraint):
# GCS also has SmartTable, Regular Language Membership, Knapsack constraints
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cpmpy's ShortTable is SmartTable

@Wout4
Copy link
Collaborator

Wout4 commented Jul 15, 2024

I couldn't find smart table in the gcs python interface, could be nice to add still. Other than that seems ready to merge!

@Wout4
Copy link
Collaborator

Wout4 commented Jul 15, 2024

Some test fail due to pycroptosat not being installed, will merge this anyway as that does not seem to happen on master

@Wout4 Wout4 marked this pull request as ready for review July 15, 2024 09:40
@Wout4 Wout4 merged commit 0cf0618 into master Jul 15, 2024
1 check failed
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.

4 participants