-
Notifications
You must be signed in to change notification settings - Fork 7
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
I actually love this paper & work -- but I'd love if the transformation was model count-preserving! #3
Comments
Demo of treewidth-reduction: git clone https://github.com/msoos/sharpsat-td/
cd sharpsat-td
git checkout preprocessor
mkdir build
cd build
rm -rf CMake* src cmake* ganak* sharp* Make*
cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DSTATIC=ON ..
make -j4
./preprocessor mc2022_track1_126.cnf > out
./sbva -i out -o out2
# now compile SSTD again, but this time normal branch
git checkout main
make -j8
# run on preprocessed instance
./sharpSAT -decot 3 -decow 100 -tmpdir . -cs 3500 out
c o Parsed 285 vars, 1695 clauses, and 0 weights.
[stuff]
c o COEF: 8.947 Width: 88
# run on preprocessed + SBVA-preprocessed instance
./sharpSAT -decot 3 -decow 100 -tmpdir . -cs 3500 out2
c o Parsed 400 vars, 787 clauses, and 0 weights.
[stuff]
c o COEF: 156.287 Width: 62 As you can see, treewidth goes from 88 -> 62. Also, SharpSAT-TD recognized that the TD is working much better in the second case, so it increased the coefficient of the TD weight, but that's kind of a side-note, it's obviously a lot better. BTW, the dual preprocessing could be disabled, but it doesn't matter. There is a significant reduction in the number of clauses, too, but that's expected of SBVA of course. The fun part is the TW (treewidth) reduction. Find mccomp track1 2022 files here: https://datashare.tu-dresden.de/index.php/s/7iZSNgKT27FEsGX This specific file is attached here: I hope the above helps in understanding the importance of SBVA for model counting, Mate |
This is experimental and untested. (see #3)
Hey @msoos, I did a quick patch that adds an option ( b0d1631 (on branch preserve_model_count) It adds one extra clause per added variable (described in a comment). This does now mean that it may perform transformations that do not reduce formula size. i.e. if it adds a 2x3 bva grid, it would remove 6 clauses, add 6 clauses (i.e. normally 5, now +1), and add 1 variable... if this turns out to be undesirable for your problems, you may want to adjust the lower bound for the reduction size here: Lines 658 to 660 in becc0e0
|
Wow, nice, thanks, that sounds fantastic! Let me test this tomorrow. I'll keep this issue open because I wanna test and report back some numbers for you so you see the difference :) |
Hi, First of all, this is great, thanks! I am hacking it a bit to have an interface. As a quick feedback regarding C++. This is something I bumped into myself and is extremely painful. When you want to enlarge a datastruct by 1 element, and you want to do this many times potentially, doing this:
Will actually re-allocate and re-copy everything (almost) every time. It's very stupid but it is what it is. Instead, you need to do this:
This will dynamically allocate more space, first allocating some exponent of 2 bytes, then if that gets used up, it allocates 2x that, etc. This way, you are waaay more efficient. In fact the 1-by-1 can take quadratic time. Sad day. Notice that the I think the above may be happening in your code here (doing it 2-by-2 I think?):
BTW, I am planning to have an API like this, I hope it's OK:
Where Config is the set of config params you can normally pass via the command line. Should be ready by tomorrow. It's working already, but I wanna have the library installed, CMake set up to automagically find local version of the compiled (but not yet installed) library, and compile on Windows, Mac, and Linux. |
Awesome! Thanks for finding the resize issue. Your changes sound great. One thing I am curious about: have you tried with |
OK, I am now done with the refactor: https://github.com/meelgroup/SBVA Changes:
You can find it here: https://github.com/meelgroup/SBVA TODO:
The system allows to be compiled for clang sanitizer, which I will do (added as a cmake flag). Perhaps there could be leaks or under/overflows. I'll hook this up into my model counting fuzzer and that should help. I will now do the above testing, add it our model counting preprocessor, and run a full performance test and see what happens. I'll keep this open to do both the checks above and the performance tests so I can get back to you. |
This is experimental and untested. (see #3)
I promised to get back to you with results, sorry it took so long. I integrated the version of SBVA that I have into my pipelines. In my minor revision of SBVA, I removed the use of the Overall, SBVA does help, though I haven't tuned it much yet. I set it to about a 10s timeout (200*1e6 steps) and I set the cutoffs here to be variable, as you suggested: if (matched_lits.size() <= config.matched_lits_cutoff &&
matched_clauses->size() <= config.matched_cls_cutoff) {
continue;
} In model counting, we don't mind to to have somewhat more clauses as long as we have less variables. So the savings need to be a bit higher than usual. I set these to 4 each, except for CDF plot is here of GANAK with/without SBVA: run.pdf Where the legend says Note that this is 2023 model counting competition, 1h timeout, 4GB RAM. The performance we have here matches that of the best solvers. |
I now checked, and regarding SBVA vs BVA -- BVA solves one problem less. So not so dramatic of a difference, but a difference nevertheless, so I'll stick with SBVA. Note that counting is a lot less noisy than a SAT check, because we have to walk the whole formula, so it's not like SAT where e.g. you can accidentally stumble on a SAT solution and be happy. It's more akin to UNSAT, although in my experience even less noisy. |
Hi,
Long story short, I would love to use this tool as a preprocessor for model counting. I could fix it I guess, but maybe you can do it faster -- I'd need it to be count-preserving. Basically, the count of the formula should not change. I think it should be possible to do, maybe not too hard? I wanna use it as a preprocessor for model counting. It turns out that SBVA can significantly reduce the treewidth of a formula, which can make it a lot more tractable for model counting. See: https://github.com/Laakeri/sharpsat-td and its associated research paper, https://drops.dagstuhl.de/entities/document/10.4230/LIPIcs.CP.2021.8
If you use this branch: https://github.com/msoos/sharpsat-td/tree/preprocessor then you can get the preprocessor of SharSAT-TD. You need to run that on a formula, the simplified CNF will be written to the console. Then either run SBVA + flowcutter or just flowcutter. You will see that the treewidth is VERY different, up to half on certain formulas. Since the time to run is ~exponential in the treewidth, SBVA could make counting sqrt of the time it takes to count, e.g. instead of 1 million seconds to count, 1000 seconds to count. Kinda neat.
Let me know what you think,
Mate
PS: I'll also make it deterministic, and into a library, and all that, because time-based timeout is non-deterministic, making debugging impossible, a hard requirement for anyone who does industrial work. Also, a library allows it to be re-usable, not only from a bash script (which again is hard to make deterministic, properly abort, etc).
@Laakeri for visibility, since SSTD is his tool
@kuldeepmeel for visibility
The text was updated successfully, but these errors were encountered: