-
Notifications
You must be signed in to change notification settings - Fork 1
Public Interface #63
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
base: dev
Are you sure you want to change the base?
Public Interface #63
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,25 @@ | ||
## Solver's interface functions | ||
|
||
Add a variable to CNF and return its number: | ||
- `addVariable(): Int` | ||
All variables are numbered in DIMACS format. | ||
|
||
Add a variable to CNF: | ||
- `newVariable()` | ||
|
||
Add a clause to CNF as pure literals or list of literals: | ||
- `addClause(literals: List<Lit>)` | ||
- `newClause(literals: List<Int>)` | ||
|
||
Solve CNF without assumptions: | ||
- `solve(): Boolean` | ||
|
||
Solve CNF with the passed assumptions: | ||
- `solve(assumptions: List<Lit>): Boolean` | ||
- `solve(assumptions: List<Int>): Boolean` | ||
|
||
Query boolean value of a literal: | ||
- `getValue(lit: Lit): Boolean` | ||
- `value(lit: Int): Boolean` | ||
|
||
**Note:** the solver should be in the SAT state. | ||
|
||
Query the satisfying assignment (model) for the SAT problem: | ||
- `fun getModel(): List<Lit>` | ||
- `fun getModel(): List<Int>` | ||
|
||
**Note:** the solver should be in the SAT state. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import org.gradle.api.tasks.testing.logging.TestExceptionFormat | ||
import org.gradle.api.tasks.testing.logging.TestLogEvent | ||
import org.jetbrains.dokka.DokkaConfiguration | ||
import org.jetbrains.dokka.model.KotlinVisibility | ||
|
||
plugins { | ||
kotlin("multiplatform") | ||
} | ||
|
||
kotlin { | ||
jvm() | ||
js { | ||
browser() | ||
} | ||
|
||
sourceSets { | ||
commonMain { | ||
dependencies { | ||
implementation(project(":core")) | ||
} | ||
} | ||
|
||
commonTest { | ||
dependencies { | ||
implementation(kotlin("test")) | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,156 @@ | ||
import org.kosat.CDCL | ||
import org.kosat.Clause | ||
import org.kosat.Lit | ||
import org.kosat.SolveResult | ||
import kotlin.math.abs | ||
|
||
/** | ||
* Wrapper for the KoSAT solver. | ||
* | ||
* All the literals and variables are in DIMACS format (positive for variables, | ||
* negative for negated variables, 1-based indexing). | ||
* | ||
* *This class is not thread-safe.* | ||
*/ | ||
class Kosat { | ||
private var numberOfVariables: Int = 0 | ||
private var numberOfClauses: Int = 0 | ||
|
||
private var solver: CDCL = CDCL() | ||
|
||
private var result: SolveResult? = null | ||
private var model: List<Boolean>? = null | ||
|
||
private fun invalidateResult() { | ||
result = null | ||
model = null | ||
} | ||
|
||
private fun checkLiterals(literals: Iterable<Int>) { | ||
for (lit in literals) { | ||
if (lit == 0) { | ||
throw IllegalArgumentException("Clause must not contain 0") | ||
} else { | ||
val varIndex = abs(lit) - 1 | ||
if (varIndex >= numberOfVariables) { | ||
throw IllegalArgumentException( | ||
"Variable of $lit is not defined (total variables: ${numberOfVariables})" | ||
) | ||
} | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Resets the solver to its initial state, removing all variables and | ||
* clauses. | ||
*/ | ||
fun reset() { | ||
numberOfVariables = 0 | ||
numberOfClauses = 0 | ||
solver = CDCL() | ||
invalidateResult() | ||
} | ||
|
||
/** | ||
* Return the number of variables in the solver. | ||
*/ | ||
fun numberOfVariables(): Int { | ||
return numberOfVariables | ||
} | ||
|
||
/** | ||
* Return the number of clauses in the solver. | ||
*/ | ||
fun numberOfClauses(): Int { | ||
return numberOfClauses | ||
} | ||
|
||
/** | ||
* Allocate a new variable in the solver. | ||
*/ | ||
fun newVariable() { | ||
invalidateResult() | ||
numberOfVariables++ | ||
solver.newVariable() | ||
} | ||
|
||
/** | ||
* Add a new clause to the solver. The literals must be in DIMACS format | ||
* (positive for variables, negative for negated variables, 1-based | ||
* indexing). All variables must be defined with [newVariable] before adding | ||
* clauses. | ||
*/ | ||
fun newClause(clause: Iterable<Int>) { | ||
checkLiterals(clause) | ||
invalidateResult() | ||
numberOfClauses++ | ||
solver.newClause(Clause.fromDimacs(clause)) | ||
} | ||
|
||
/** | ||
* Add a new clause to the solver. The literals must be in DIMACS format | ||
* (positive for variables, negative for negated variables, 1-based | ||
* indexing). All variables must be defined with [newVariable] before adding | ||
* clauses. | ||
*/ | ||
fun newClause(vararg literals: Int) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the users that already have an IntArray of literals? Are we going to force them to use |
||
newClause(literals.asIterable()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You probably don't want to allow users override this method. |
||
} | ||
|
||
/** | ||
* Solve the SAT problem with the given assumptions, if any. Assumptions are | ||
* literals in DIMACS format. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only "vararg" version of |
||
*/ | ||
fun solve(vararg assumptions: Int): Boolean { | ||
return solve(assumptions.asIterable()) | ||
} | ||
|
||
/** | ||
* Solve the SAT problem with the given assumptions | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Document the return type. |
||
*/ | ||
fun solve(assumptions: Iterable<Int>): Boolean { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about UNKNOWN results? Budget exhaustion and async interrupts might lead to the solver state with is neither SAT (has a satisfying model) nor UNSAT (truly proved unsatisfiability). |
||
checkLiterals(assumptions) | ||
result = solver.solve(assumptions.map { Lit.fromDimacs(it) }) | ||
return result == SolveResult.SAT | ||
} | ||
|
||
/** | ||
* If the problem is SAT, return the model as a list of booleans. [solve] | ||
* must be called before calling this method. The model is cached after the | ||
* first call and reset when clause or variable is incrementally is added. | ||
*/ | ||
fun getModel(): List<Boolean> { | ||
if (result == null) { | ||
throw IllegalStateException("Model is not available before solving") | ||
} else if (result != SolveResult.SAT) { | ||
throw IllegalStateException("Model is not available if result is not SAT") | ||
} | ||
|
||
return solver.getModel().also { model = it } | ||
} | ||
|
||
/** | ||
* If the problem is SAT, return the value of the given literal in the | ||
* model. [solve] must be called before calling this method. | ||
*/ | ||
fun value(literal: Int): Boolean { | ||
if (literal == 0) { | ||
throw IllegalArgumentException("Literal must not be 0") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be written via |
||
} | ||
|
||
val varIndex = abs(literal) - 1 | ||
|
||
if (varIndex >= numberOfVariables) { | ||
throw IllegalArgumentException( | ||
"Variable of $literal is not defined (total variables: ${numberOfVariables})" | ||
) | ||
} | ||
|
||
if (model == null) { | ||
model = getModel() | ||
} | ||
|
||
return model!![varIndex] xor (literal < 0) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
import kotlin.test.Test | ||
import kotlin.test.assertEquals | ||
import kotlin.test.assertFailsWith | ||
import kotlin.test.assertFalse | ||
import kotlin.test.assertTrue | ||
|
||
class KotlinApiTest { | ||
@Test | ||
fun tieShirtVarargs() { | ||
val solver = Kosat() | ||
|
||
assertFailsWith(IllegalArgumentException::class) { solver.newClause(1, 2) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about |
||
|
||
solver.newVariable() | ||
solver.newVariable() | ||
|
||
solver.newClause(-1, 2) | ||
solver.newClause(1, 2) | ||
solver.newClause(-1, -2) | ||
|
||
assertFailsWith(IllegalStateException::class) { solver.getModel() } | ||
assertFailsWith(IllegalStateException::class) { solver.value(1) } | ||
assertFailsWith(IllegalArgumentException::class) { solver.value(0) } | ||
assertFailsWith(IllegalArgumentException::class) { solver.value(-5) } | ||
|
||
assertTrue(solver.solve()) | ||
|
||
assertFalse(solver.value(1)) | ||
assertTrue(solver.value(-1)) | ||
assertTrue(solver.value(2)) | ||
assertFalse(solver.value(-2)) | ||
|
||
assertFailsWith(IllegalArgumentException::class) { solver.value(0) } | ||
assertFailsWith(IllegalArgumentException::class) { solver.value(-5) } | ||
|
||
assertEquals(listOf(false, true), solver.getModel()) | ||
|
||
assertTrue(solver.solve(-1)) | ||
assertFalse(solver.solve(1)) | ||
assertTrue(solver.solve(-1, 2)) | ||
assertFalse(solver.solve(-1, -2)) | ||
|
||
solver.newClause(1, -2) | ||
assertFalse(solver.solve()) | ||
|
||
assertFailsWith(IllegalStateException::class) { solver.getModel() } | ||
|
||
solver.reset() | ||
|
||
assertFailsWith(IllegalStateException::class) { solver.getModel() } | ||
|
||
solver.newVariable() | ||
solver.newVariable() | ||
solver.newVariable() | ||
|
||
assertTrue(solver.solve()) | ||
|
||
assertEquals(3, solver.numberOfVariables()) | ||
assertEquals(0, solver.numberOfClauses()) | ||
assertEquals(3, solver.getModel().size) | ||
} | ||
|
||
@Test | ||
fun tieShirtIterables() { | ||
val solver = Kosat() | ||
|
||
solver.newVariable() | ||
solver.newVariable() | ||
|
||
solver.newClause(listOf(-1, 2)) | ||
solver.newClause(setOf(1, 2)) | ||
solver.newClause(listOf(-1, -2)) | ||
|
||
assertTrue(solver.solve()) | ||
|
||
assertEquals(listOf(false, true), solver.getModel()) | ||
|
||
assertTrue(solver.solve(setOf(-1))) | ||
assertFalse(solver.solve(listOf(1))) | ||
assertTrue(solver.solve(listOf(-1, 2))) | ||
assertFalse(solver.solve(setOf(-1, -2))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about testing the |
||
|
||
solver.newClause(listOf(1, -2)) | ||
assertFalse(solver.solve()) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,3 +19,4 @@ fun myInclude(name: String) { | |
myInclude("core") | ||
myInclude("cli") | ||
myInclude("web") | ||
myInclude("api") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "wrapper", see the first comment in the review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The module naming (and splitting, in the first place) is weird. API should NOT depend on the implementation, but the "implementation" should implement the public interface contract. Note that the "implementation" has a separate choice of making API dependency truly
api
(in Gradle terms,api
vsimplementation
configuration), however it is not "mandatory": the implementation might choose to comply with the interface without actually letting us know about it.One example of a "good" api/impl split is SLF4J and its implementations (e.g. slf4j-simple). The same goes for log4j, if you prefer it.
That way, ideally, if the user only wants the interface, it declares the dependency on
api
module and receives only that - bare interface. However, when the user wants some kinda implementation, it declares the dependency on "core"/"impl" module, and receives the working classes. If he wants to interact with this implementation via some fancy interface, it should either additionally declare a dependency on API-providing module, or force "core"/"impl" to provide it viaapi
Gradle-configuration.Currently, your "kosat-api" module provides a mere "fancy wrapper" for CDCL, not an actual API.
See https://github.com/Lipen/sat-nexus/blob/master/lib/wrappers/src/wrap_minisat.rs
All in all, I would suggest NOT splitting impl into "core" and "wrapper", since it requires the duplicate multiplatform setup, and does not provide any value. Simply provide both "internal solver" (
CDCL
) and "external solver" (KoSAT
) in the same module (and even in the same packageorg.kosat.*
).