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

Types specifying the simplify-implication RPC endpoint #3614

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
121 changes: 121 additions & 0 deletions docs/2022-07-18-JSON-RPC-Server-API.md
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,127 @@ Same as for execute
}
```

## Simplify Implication

### Request:

```json
{
"jsonrpc": "2.0",
"id": 1,
"method": "simplify-implication",
"params": {
"antecedent": {"format": "KORE", "version": 1, "term": {}},
"consequent": {"format": "KORE", "version": 1, "term": {}},
"module": "MODULE-NAME"
}
}
```

Optional parameters: `module` (main module name)

The request format is shared with the `"implies"` method.

**NOTE**: `"simplify-implication"` currently only has a stub implementation in `kore-rpc`. The real implementation is in `kore-rpc-booster` (see [hs-backend-booster](https://github.com/runtimeverification/hs-backend-booster) repository). The documentation will reside here for consistency.

### Error Response:

Errors in decoding the `antecedent` or `consequent` terms are reported similar as for execute.

### Correct Response:

The endpoint is a lightweight variant fo the `"implies"` endpoint, which checks implication using matching between configuration terms and a lightweight (using K simplifications, rather than encoding to SMT) constraint subsumption.

The implication can be "valid", "invalid" and "unknown". The result is returned in the `"validity"` field. The following results are possible:

#### Implication is **valid**

A constrained substitution as the result, and this is only returned if the implication is valid.

```
{
"jsonrpc": "2.0",
"id": 1,
"result": {
"validity": {"tag": "implication-valid"},
"substitution": {"format": "KORE", "version": 1, "term": {}},
}
}
```

#### Implication **invalid**: terms do not match

Matching between antecedent and consequent configurations failed (different constructors, shared variables, sort error, etc.), constraints has not been subsumption been attempted. No matching substitution is returned.

```
{
"jsonrpc": "2.0",
"id": 1,
"result": {
"validity": {"tag": "implication-valid",
"contents": {"tag": "matching-failed",
"contents": "Shared variables: X:SortWordStack{}"
}},
}
}
```
Comment on lines +439 to +454
Copy link
Member

Choose a reason for hiding this comment

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

The result here should be "unification-failed", and that's what we should be checking. Eventually, if unification succeeds and matching fails, we want ot return an core problem which may or may not be unsat, that the user can then forward to the more powerful implies endpoint.

Just want to make sure that when we get this response back, what it actually means is there is absolutely no overlap between the terms, we know because we found differing constructors.


#### Implication **invalid**: terms match, but constraints subsumption failed

Matching between antecedent and consequent configurations is successful, but constraints do not agree. Response contains the matching substitution and the unsatisfiable core of constraints.

```
{
"jsonrpc": "2.0",
"id": 1,
"result": {
"validity": {"tag": "implication-valid",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"validity": {"tag": "implication-valid",
"validity": {"tag": "implication-invalid",

I think this should likely actually just be unknown result actually, but we return the unsat core (the constarints we arent' sure about).

"contents": {"tag": "constraint-subsumption-failed",
"contents": {"format": "KORE", "version": 1, "term": {}}
}},
"substitution": {"format": "KORE", "version": 1, "term": {}},
}
}
```

#### Implication **unknown**: matching indeterminate

The matching algorithm is incomplete and may return an indeterminate result. The response will contains the subterms that the algorithm could not know how to match. No substitution is returned.

```
{
"jsonrpc": "2.0",
"id": 1,
"result": {
"validity": {"tag": "implication-unknown",
"contents": {"tag": "matching-unknown",
"contents": {"first" : {"format": "KORE", "version": 1, "term": {}}
,"second" : {"format": "KORE", "version": 1, "term": {}}
}
}},
}
}
```

#### Implication **unknown**: constraint subsumption indeterminate

If matching is successful, but the constraint solver procedure (internal, or the SMT solver if enabled) returned "unknown". Response contains the matching substitution and the unknown core of constraints.

```
{
"jsonrpc": "2.0",
"id": 1,
"result": {
"validity": {"tag": "implication-unknown",
"contents": {"tag": "constraint-subsumption-unknown",
"contents": {"tag": "constraint-subsumption-failed",
"contents": {"format": "KORE", "version": 1, "term": {}}
}},
"substitution": {"format": "KORE", "version": 1, "term": {}},
}
}
```
Comment on lines +493 to +510
Copy link
Member

Choose a reason for hiding this comment

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

How is this different than "Implication invalid: terms match, but constraints subsumption failed"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought, we'd want to have a tri-state constraint subsumption check, just like we have a tri-stat matching routine. But perhaps we can indeed expose the "failed" and "unknown" constraint subsumption as just "failed" and have less user-facing cases.

Copy link
Member

Choose a reason for hiding this comment

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

Part of the confusion for me is that for the failure cases, you're using "terms don't match" as the criteria, but that's not how the tri-state matching algorithm works. It either reports back:

  • Terms match with substitution alpha.
  • Terms definitely don't unify.
  • Unknown.

The key here is that unification and matching are different algorithms, different complexities, and different properties. Saying a term matches another means it's completely subsumed, and saying that they don't unify means they have no overlap.

So I guess the language here needs to be cleared up, because here for example
image
we're saying "terms don't match" because of differing constructor, where I think it should be saying "terms don't unify".

Unification and matching are not interchangeable. See https://github.com/runtimeverification/hs-backend-booster/issues/193 again for the desired interface.


## Add-module

### Request
Expand Down
49 changes: 49 additions & 0 deletions kore-rpc-types/src/Kore/JsonRpc/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ instance FromRequest (API 'Req) where
parseParams "simplify" = Just $ fmap (Simplify <$>) parseJSON
parseParams "add-module" = Just $ fmap (AddModule <$>) parseJSON
parseParams "get-model" = Just $ fmap (GetModel <$>) parseJSON
parseParams "simplify-implication" = Just $ fmap (SimplifyImplication <$>) parseJSON
parseParams "cancel" = Just $ const $ return Cancel
parseParams _ = Nothing

Expand Down Expand Up @@ -177,6 +178,48 @@ data GetModelResult = GetModelResult
(FromJSON, ToJSON)
via CustomJSON '[OmitNothingFields, FieldLabelModifier '[CamelToKebab]] GetModelResult

data SimplifyImplicationResult = SimplifyImplicationResult
{ validity :: ImplicationValidityResult
, substitution :: Maybe KoreJson
, logs :: Maybe [LogEntry]
}
deriving stock (Generic, Show, Eq)
deriving
(FromJSON, ToJSON)
via CustomJSON '[OmitNothingFields, FieldLabelModifier '[CamelToKebab]] SimplifyImplicationResult

data ImplicationValidityResult
= -- | implication is valid
ImplicationValid
| -- | implication is invalid, explains why
ImplicationInvalid ImplicationInvalidReason
| -- | implication is unknown, explains why
ImplicationUnknown ImplicationUnknownReason
deriving stock (Generic, Show, Eq)
deriving
(FromJSON, ToJSON)
via CustomJSON '[ConstructorTagModifier '[CamelToKebab]] ImplicationValidityResult

data ImplicationInvalidReason
= -- | antecedent and consequent do not match
MatchingFailed Text
| -- | matching was successful, but constraints don't agree: return unsatisfiable core of constraints
ConstraintSubsumptionFailed KoreJson
deriving stock (Generic, Show, Eq)
deriving
(FromJSON, ToJSON)
via CustomJSON '[ConstructorTagModifier '[CamelToKebab]] ImplicationInvalidReason

data ImplicationUnknownReason
= -- | matching between antecedent and consequent is indeterminate, return the subterms that caused this
MatchingUnknown KoreJson KoreJson
| -- | matching was successful, but constraint subsumption is indeterminate: return unknown constraints
ConstraintSubsumptionUnknown KoreJson
deriving stock (Generic, Show, Eq)
deriving
(FromJSON, ToJSON)
via CustomJSON '[ConstructorTagModifier '[CamelToKebab]] ImplicationUnknownReason

data SatResult
= Sat
| Unsat
Expand All @@ -194,6 +237,7 @@ data APIMethod
| SimplifyM
| AddModuleM
| GetModelM
| SimplifyImplicationM
deriving stock (Eq, Ord, Show, Enum)

type family APIPayload (api :: APIMethod) (r :: ReqOrRes) where
Expand All @@ -207,13 +251,16 @@ type family APIPayload (api :: APIMethod) (r :: ReqOrRes) where
APIPayload 'AddModuleM 'Res = ()
APIPayload 'GetModelM 'Req = GetModelRequest
APIPayload 'GetModelM 'Res = GetModelResult
APIPayload 'SimplifyImplicationM 'Req = ImpliesRequest
APIPayload 'SimplifyImplicationM 'Res = SimplifyImplicationResult

data API (r :: ReqOrRes) where
Execute :: APIPayload 'ExecuteM r -> API r
Implies :: APIPayload 'ImpliesM r -> API r
Simplify :: APIPayload 'SimplifyM r -> API r
AddModule :: APIPayload 'AddModuleM r -> API r
GetModel :: APIPayload 'GetModelM r -> API r
SimplifyImplication :: APIPayload 'SimplifyImplicationM r -> API r
Cancel :: API 'Req

deriving stock instance Show (API 'Req)
Expand All @@ -228,6 +275,7 @@ instance ToJSON (API 'Res) where
Simplify payload -> toJSON payload
AddModule payload -> toJSON payload
GetModel payload -> toJSON payload
SimplifyImplication payload -> toJSON payload

instance Pretty.Pretty (API 'Req) where
pretty = \case
Expand All @@ -236,6 +284,7 @@ instance Pretty.Pretty (API 'Req) where
Simplify _ -> "simplify"
AddModule _ -> "add-module"
GetModel _ -> "get-model"
SimplifyImplication _ -> "simplify-implication"
Cancel -> "cancel"

rpcJsonConfig :: PrettyJson.Config
Expand Down
8 changes: 7 additions & 1 deletion kore/src/Kore/JsonRpc.hs
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,13 @@ respond serverState moduleName runSMT =
PatternJson.fromSubstitution sort $
Substitution.mapVariables getRewritingVariable subst
}

SimplifyImplication ImpliesRequest{antecedent, consequent} ->
pure . Right . SimplifyImplication $
SimplifyImplicationResult
{ validity = ImplicationUnknown (MatchingUnknown antecedent consequent)
, substitution = Nothing
, logs = mempty
}
-- this case is only reachable if the cancel appeared as part of a batch request
Cancel -> pure $ Left cancelUnsupportedInBatchMode
where
Expand Down
18 changes: 18 additions & 0 deletions test/rpc-server/runTests.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,3 +254,21 @@ def runTest(def_path, req, resp_golden_path):
params["state"] = state
req = rpc_request_id1("get-model", params)
runTest(get_model_def_path, req, resp_golden_path)

print("Running simplify-implication tests:")

for name in os.listdir("./simplify-implication"):
info(f"- test '{name}'...")
simplify_implication_def_path = os.path.join("./simplify-implication", name, "definition.kore")
params_json_path = os.path.join("./simplify-implication", name, "antecedent.json")
state_json_path = os.path.join("./simplify-implication", name, "consequent.json")
resp_golden_path = os.path.join("./simplify-implication", name, "response.golden")
with open(params_json_path, 'r') as antecedent_json:
with open(state_json_path, 'r') as consequent_json:
antecedent = json.loads(antecedent_json.read())
consequent = json.loads(consequent_json.read())
params = {}
params["antecedent"] = antecedent
params["consequent"] = consequent
req = rpc_request_id1("simplify-implication", params)
runTest(simplify_implication_def_path, req, resp_golden_path)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`X => X`, response `ImplicationUnknown`, with empty substitution. `simplify-implication` is only implemented as a stub in `kore-rpc`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"format": "KORE",
"version": 1,
"term": {
"tag": "EVar",
"name": "X",
"sort": {
"tag": "SortVar",
"name": "S"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"format": "KORE",
"version": 1,
"term": {
"tag": "EVar",
"name": "X",
"sort": {
"tag": "SortVar",
"name": "S"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"jsonrpc":"2.0","id":1,"result":{"validity":{"tag":"implication-unknown","contents":{"tag":"matching-unknown","contents":[{"format":"KORE","version":1,"term":{"tag":"EVar","name":"X","sort":{"tag":"SortVar","name":"S"}}},{"format":"KORE","version":1,"term":{"tag":"EVar","name":"X","sort":{"tag":"SortVar","name":"S"}}}]}}}}