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

Empty Clauses #15

Closed
wants to merge 1 commit into from
Closed

Empty Clauses #15

wants to merge 1 commit into from

Conversation

sacado
Copy link

@sacado sacado commented Jul 2, 2015

Modified pigosat.AddClauses so that empty clauses are no longer ignored and make the formula unsatisfiable; modified tests consequently.

…ed and make the formula UNSAT ; modified tests consequently
@@ -213,7 +221,7 @@ func wasExpected(t *testing.T, i int, p *Pigosat, ft *formulaTest,
p.AddedOriginalClauses())
}
if s := p.Seconds(); s <= 0 || s > time.Millisecond {
t.Errorf("Test %d: Test took a suspicious amount of time: %v", i, s)
t.Logf("Test %d: Test took a suspicious amount of time: %v", i, s)
Copy link
Owner

Choose a reason for hiding this comment

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

This change does not seem germaine. What was your reasoning?

@wkschwartz wkschwartz changed the title Modified pigosat.AddClauses so that empty clauses are no longer ignor… Empty Clauses Jul 7, 2015
@@ -223,6 +223,8 @@ func (p *Pigosat) AddClauses(clauses Formula) {
for _, clause := range clauses {
count = len(clause)
if count == 0 {
// Empty clause: add a clause with only literal 0
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this comment is very informative. You can go ahead and kill it.

@wkschwartz
Copy link
Owner

Please amend the docstrings for AddClauses and for type Clause to say that "Empty clauses are no ignored and make a formula unsatisfiable."

@wkschwartz
Copy link
Owner

Ref #14

@wkschwartz
Copy link
Owner

@sacado My goal is to merge this PR once #12 is merged. Would you mind hitting Edit on the PR and click "Allow edits from maintainers"? This will allow me to change some minor things without having to send a PR to your branch first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants