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

improve the error message about expanding powersets #2969

Merged
merged 5 commits into from
Aug 25, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .unreleased/bug-fixes/2969-powset-expansion
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Better error reporting for hitting the limits of `SUBSET` expansion, see #2969
5 changes: 5 additions & 0 deletions test/tla/TestSets.tla
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ TestExists15InPowerset ==
\E S \in SUBSET Set263:
S = { 2, 6 }

\* This test is expected to fail. It should be run separately.
FailExpandLargePowerset ==
\E S \in SUBSET (1..30):
31 \notin S

TestForallInPowerset ==
\A S \in SUBSET Set1357:
6 \notin S
Expand Down
10 changes: 10 additions & 0 deletions test/tla/cli-integration-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -2668,6 +2668,16 @@ $ apalache-mc check --length=0 --inv=AllTests TestSets.tla | sed 's/[IEW]@.*//'
EXITCODE: OK
```

### check TestSets.tla reports an error on FailExpandLargePowerset

```sh
$ apalache-mc check --length=0 --inv=FailExpandLargePowerset TestSets.tla | sed 's/[IEW]@.*//'
...
<unknown>: known limitation: Attempted to expand SUBSET of size 2^30, whereas the built-in limit is 1048576
...
EXITCODE: ERROR (255)
```

### check TestCommunityFunctions.tla reports no error (array-encoding)

```sh
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ object Limits {
val MAX_PRODUCT_SIZE = 1000000

/**
* An upper bound on the size of an expanded powerset.
* An upper bound on the size of an expanded powerset, currently, `2^20`.
*/
val POWSET_MAX_SIZE = 1000000
val POWSET_MAX_SIZE = 1048576

/**
* An upper bound on the number of rewriting steps that are applied to the same rule.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ class CheckerExceptionAdapter @Inject() (sourceStore: SourceStore, changeListene
"%s: no rule to rewrite a TLA+ expression: %s".format(findLoc(err.causeExpr.ID), err.getMessage)
FailureMessage(msg)

case err: RewriterKnownLimitationError =>
val msg = "%s: known limitation: %s".format(findLoc(err.causeExpr.ID), err.getMessage)
NormalErrorMessage(msg)

case err: RewriterException =>
val msg = "%s: rewriter error: %s".format(findLoc(err.causeExpr.ID), err.getMessage)
FailureMessage(msg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ class CheckerException(message: String, val causeExpr: TlaEx) extends Exception(
*/
class RewriterException(message: String, causeExpr: TlaEx) extends CheckerException(message, causeExpr)

/**
* This exception is thrown when the rewriter meets a generally well-formed TLA+ expression that cannot be handled due
* to some known limitations of the model checker.
*
* @param message
* error message
* @param causeExpr
* the problematic TLA+ expression, may be NullEx
*/
class RewriterKnownLimitationError(message: String, causeExpr: TlaEx) extends CheckerException(message, causeExpr)

/**
* This exception is thrown when QStateRewrite cannot find an applicable rule.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package at.forsyte.apalache.tla.bmcmt.rules.aux
import at.forsyte.apalache.tla.bmcmt._
import at.forsyte.apalache.tla.lir.SetT1
import at.forsyte.apalache.tla.types.{tlaU => tla}
import com.typesafe.scalalogging.LazyLogging

/**
* This class constructs the power set of S, that is, SUBSET S. Sometimes, this is just unavoidable, e.g., consider { Q
Expand All @@ -11,7 +12,7 @@ import at.forsyte.apalache.tla.types.{tlaU => tla}
* @author
* Igor Konnov
*/
class PowSetCtor(rewriter: SymbStateRewriter) {
class PowSetCtor(rewriter: SymbStateRewriter) extends LazyLogging {

// Confringo is the explosion curse from Harry Potter. To let you know that your SMT solver will probably explode.
def confringo(state: SymbState, set: ArenaCell): SymbState = {
Expand Down Expand Up @@ -40,8 +41,13 @@ class PowSetCtor(rewriter: SymbStateRewriter) {

rewriter.solverContext.log("; %s(%s) {".format(getClass.getSimpleName, state.ex))
val powSetSize = BigInt(1) << elems.size
if (powSetSize >= Limits.POWSET_MAX_SIZE) {
throw new RewriterException(s"Attempted to expand a powerset of size 2^${elems.size}", state.ex)
if (powSetSize > Limits.POWSET_MAX_SIZE) {
logger.error("This error typically occurs when one enumerates all subsets:"
+ " \\A S \\in SUBSET T: P or \\E S \\in SUBSET T: P")
logger.error("Try to decrease the cardinality of the base set T, or avoid enumeration.")
val msg =
s"Attempted to expand SUBSET of size 2^${elems.size}, whereas the built-in limit is ${Limits.POWSET_MAX_SIZE}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Printing 1048576 here may look confusing to the user.

Why not store Limits.POWSET_MAX_SIZE as the number of elements?
Then you can directly compare elems.size above and print it here as a power of 2.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's true. I will fix that :)

throw new RewriterKnownLimitationError(msg, state.ex)
}
val subsets =
if (elems.nonEmpty) {
Expand Down
Loading