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.

Sign up for GitHub

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

Jump to bottom

Multi-thread trigger assertion #38

Closed
Flash-LHR opened this issue Jan 30, 2023 · 5 comments
Closed

Multi-thread trigger assertion #38

Flash-LHR opened this issue Jan 30, 2023 · 5 comments

Comments

@Flash-LHR
Copy link
Contributor

Flash-LHR commented Jan 30, 2023 •
edited
Loading

image

The operation succeeds when using the default number of threads, but the assertion fails after setting the number of threads.
OS： 5.17.0-kali3-amd64
CPU：Intel(R) Core(TM) i5-8300H CPU @ 2.30GHz

r05.cnf.txt
@msoos
Copy link
Collaborator

msoos commented Jan 30, 2023

Hi,

Thanks! Is this related to this? https://github.com/meelgroup/bosphorus/issues In other words, does that PR fix this bug or it causes this bug? I understand that this needs fixing, but want to understand first how to reproduce.

Cheers,

Mate

@Flash-LHR
Copy link
Contributor Author

Flash-LHR commented Jan 30, 2023 •
edited
Loading

This issue has nothing to do with pr.

Pr only sets the number of threads of cryptominisat5. Before pr, the number of threads of cryptominisat5 is always 1 regardless of whether the user specifies the thread parameter. In fact, I did not attempt to fix the above assertion failure.

I have uploaded the file r05.cnf for reproducing.

msoos added a commit to msoos/cryptominisat that referenced this issue Feb 5, 2023
@msoos
Fixing issue with calling inprocess strategy
9b4ee6f 
Fixes issue:

meelgroup/bosphorus#38

by @Flash-LHR on GitHub
@msoos
Copy link
Collaborator

msoos commented Feb 5, 2023

Hi,

Sorry for the late response. It was indeed a bug! I fixed it in CryptoMiniSat, here: msoos/cryptominisat@9b4ee6f

If you recompile both CMS and Bosphorus (maybe no need to recompile Bosphorus, depending on your setup), it should now work! I have tested it :)

I would like to thank you for both the perfectly reproducible problem description and your patience. Sorry, lately I've been very busy so I couldn't get to this as soon as I hoped. Nevertheless, about 1 week to fix is probably not too bad. Thanks again for the bug report, it really helped,

Mate

@msoos msoos closed this as completed Feb 5, 2023
@Flash-LHR
Copy link
Contributor Author

Thank you very much, it helps me a lot.

Besides, I have made a pr to set threadNum for cryptominisat in bosphorus, maybe you can consider that.

@msoos
Copy link
Collaborator

msoos commented Feb 15, 2023

Ah, good point, thanks for the reminder! I merged the PR as well, and cleaned up the code a bit along the way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@msoos @Flash-LHR