Skip to content

Add PySAT support, add .solve(), make .models() lazy - #19

Merged
haz merged 21 commits into
masterfrom
pysat
Aug 23, 2020
Merged

haz merged 21 commits into
masterfrom
pysat

Conversation

@blyxxyz

@blyxxyz blyxxyz commented Aug 18, 2020

Copy link
Copy Markdown
Collaborator
  • Add PySAT support. This is an optional dependency. If installed it's used by default for satisfiability and solving, except for very small sentences. The overhead is low and it seems reliable.
    It's not used by default for model enumeration, because PySAT's implementation is naive and ends up slower than the native solver in many cases.
    Resolves Optionally call pysat if it is installed #10.
  • Add a general configuration system. It uses a few more weird Python tricks than ideal, but I think it ended up pretty robust and pleasant to work with.
  • Implement .solve(). Resolves Method that returns an arbitrary model (.solve()) #15.
  • Make all implementations of model enumeration lazy. The one for DNNF ended up nicer than before, but the one for d-DNNF uses some more awful Python tricks that I'm not entirely happy with.
  • Remove the Builder class. It didn't actually solve the problem it was supposed to.
  • Various other tweaks and fixes.

@blyxxyz
blyxxyz requested a review from haz August 18, 2020 21:18

@haz haz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Seems like the tests are failing for 3.4 / 3.4, ya?

Seems pretty good to me. Magic # of 5 on the sentence size for auto choosing the SAT backend? ;). What were the performance details you found?

Comment thread nnf/dimacs.py Resolved
Comment thread nnf/pysat.py Resolved
Comment thread nnf/util.py Resolved
Comment thread test_nnf.py Resolved
Comment thread nnf/__init__.py Resolved
Comment thread nnf/__init__.py Resolved
Comment thread nnf/__init__.py Resolved
@blyxxyz

blyxxyz commented Aug 22, 2020

Copy link
Copy Markdown
Collaborator Author

Magic # of 5 on the sentence size for auto choosing the SAT backend? ;). What were the performance details you found?

I played around with generated CNF sentences, and this was the relatively worst performance for the native backend on small sentences I found:

image
(number of clauses on the x axis, time in seconds on the y axis)

I could move the intersection to the right with different parameters, but not further left.

@haz
haz merged commit 57c8a33 into master Aug 23, 2020
@blyxxyz
blyxxyz deleted the pysat branch August 23, 2020 12:34
@blyxxyz

blyxxyz commented Sep 11, 2020

Copy link
Copy Markdown
Collaborator Author

When/right before releasing 0.3.0 the README should be updated to recommend pip install nnf[pysat], to pull in PySAT.

If we do it too soon then the instructions people find on Github don't work yet and if we do it after then the outdated instructions will end up on PyPI.

@haz

haz commented Sep 15, 2020

Copy link
Copy Markdown
Collaborator

I think I'm about ready to pull the trigger on 0.3.0. Any last-minute changes you wanted to throw in?

@blyxxyz

blyxxyz commented Sep 15, 2020

Copy link
Copy Markdown
Collaborator Author

No, looks good to me.

@haz

haz commented Sep 16, 2020

Copy link
Copy Markdown
Collaborator

Hope it was done right, but we should be all set.

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.

Method that returns an arbitrary model (.solve()) Optionally call pysat if it is installed

2 participants