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

Panic at CreuSAT/src/parser.rs:130:14 on simple formula #44

Open
wintered opened this issue Aug 27, 2024 · 4 comments
Open

Panic at CreuSAT/src/parser.rs:130:14 on simple formula #44

wintered opened this issue Aug 27, 2024 · 4 comments

Comments

@wintered
Copy link

wintered commented Aug 27, 2024

b36aacd

$./CreuSAT --file f.cnf                                                            
c Reading file 'f.cnf'                                                             
c Parsed formula with 1 clauses and 1 literals                                     
thread 'main' panicked at CreuSAT/src/parser.rs:130:14:                            
Sarek should really make the parser non-binary                                     
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace   
$minisat f.cnf|tail -n 1                                                           
SATISFIABLE                                                                        
$cat f.cnf                                                                         
cnf 1 1                                                                            
-1 1 0                                                                            
@sarsko
Copy link
Owner

sarsko commented Aug 27, 2024

Hi, thank you.

This does not look like a valid cnf formula to me — there is no p line. I believe it should work if cnf 1 1 is substituted with p cnf 1 1

@sarsko
Copy link
Owner

sarsko commented Aug 27, 2024

(and yes the parser should ideally not panic. I have never bothered to make a very nice parser, and I also believe the current one has some bugs. I did make a nicer parser for a different project at some point, but never got around to including it here.)

@wintered
Copy link
Author

wintered commented Aug 27, 2024

Thanks @sarsko for the speedy reply; the corrected formula still leads to the panic.

$CreuSAT --file f.cnf 
c Reading file 'f.cnf'
c Parsed formula with 1 clauses and 1 literals
thread 'main' panicked at CreuSAT/src/parser.rs:130:14:
Sarek should really make the parser non-binary
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
$cat f.cnf 
p cnf 1 1
-1 1 0
$

We (+ @SunHao-0) had a quick look. The issue might be related to the invariants in src/clause.rs. I.e., src/clause.rs:no_duplicates returns false for this and other formulas where the positive and negative literals of the same variable share a clause.

@sarsko
Copy link
Owner

sarsko commented Aug 27, 2024

Oh, indeed, I didn't pay attention to the error line. Looks like the first formula is also accepted by the parser.

Hmm, yes, I now remember. I added invariant establishing with a mix of needed invariants and convenient invariants, but never bothered to add and prove "formula fixing" (eg for this case remove the clause, for duplicated literals reduce the clause to only contain one of the literals) for the convenient ones. The idea was to at some point go and implement a better "mid-layer" with that + some actual preprocessing stuff, but well, I never got around to it.

Dunno where that leaves this issue. Thanks, and sorry. I'd like to improve this, but not sure I will anytime soon.

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