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

Incorrect output? #15

Closed
alexeyignatiev opened this issue Oct 16, 2023 · 8 comments
Closed

Incorrect output? #15

alexeyignatiev opened this issue Oct 16, 2023 · 8 comments

Comments

@alexeyignatiev
Copy link

Hi guys,

I was trying to provide a simple interface to pyunigen from PySAT and so I checked the API. In particular, I tried to reproduce the example from the README (which is also listed here). As far as I can see, the output I get contains empty samples. So I wonder if I am doing something incorrectly or whether there is an issue in the current version:

import pyunigen
c = pyunigen.Sampler()
c.add_clause([1, 5])
c.add_clause([10, 11, 12])
c.sample(num=2, sampling_set=range(1, 5))
(16, 0, [[], []])

My understanding is that the first 2 outputs come directly from ApproxMC while the third output, which is supposed to contain the list of samples, should be of interest to pyunigen's user. Somehow no matter the clauses I try, this list always contains num empty samples.

Best,
Alexey

@msoos
Copy link
Collaborator

msoos commented Oct 16, 2023

Yep, this indeed is a bug! Thank you so much for reporting it. Sorry! I'm working on it :) Also, it should (and all of CryptoMiniSat, ApproxMC, and UniGen) should now also build a Windows pypi package, thanks to @alexmfrey

@msoos
Copy link
Collaborator

msoos commented Oct 16, 2023

Nice bug :D But I figured it out in the end! The pushed new version should be good. I will check tomorrow (gotta go and sleep now), and then release a new version to PyPi too. The new output is:

(16, 0, [[1, 2, -3, -4], [-1, -2, 3, 4]])

Which I think is OK. Let me know if this fixes it on your side. All should build on Windows too.

Beware, by the way -- pyunigen is not using Arjun currently. This means that it's a LOT slower than ApproxMC for counting. I'll fix that in the coming week or so.

@alexeyignatiev
Copy link
Author

Thanks, Mate. I will check this on my end when the new version is pushed to PyPI. Regarding the lack of Arjun in pyunigen, I am no rush with this - I just wanted to finally add the interface to your tools from PySAT, which is something you asked me about long ago. 🙂

@msoos
Copy link
Collaborator

msoos commented Oct 17, 2023

New pypi module has been released. Also, it compiles on Windows, too, and we have a Windows pypi wheel as well now. I think I can close this now :) Let me know if it helped! I'd be VERY happy to help with any issues you have :)

Thanks again for the bug report and for including it in PySAT,

Mate

@msoos msoos closed this as completed Oct 17, 2023
@alexeyignatiev
Copy link
Author

Thanks, Mate. I will give the new version a try!

@msoos
Copy link
Collaborator

msoos commented Nov 6, 2023

Hey hey! How did it go? :)

@alexeyignatiev
Copy link
Author

Sorry, got snowed under with piles of work! 😢 Will have to check later.

@msoos
Copy link
Collaborator

msoos commented Nov 8, 2023

Nice, thanks!

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

No branches or pull requests

2 participants