Retrievable explanations, Restrict explanations to ConceptMaps #20
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
haikalpribadi
approved these changes
Oct 29, 2019
This was referenced Oct 29, 2019
flyingsilverfin
added a commit
to typedb/typedb
that referenced
this pull request
Nov 1, 2019
## What is the goal of this PR? Large explanations can break gRPC message limits, specifically nesting depth when returning deep explanation trees. With this change, each layer of the explanation tree can be retrieved from the server separately. This reflects the changes in the protocol here: typedb/typedb-protocol#20. We retain the storage of Explanation trees on the Server, and expose a new RPC message with corresponding handler on the server to retrieve particular layers of the tree using the query pattern provided by the client. The query pattern is used to access the query cache directly. Additionally, `pattern` in `ConceptMap`s now contain the IDs indicated in the actual concept map as well. Finishes #4605, and #4545 ## What are the changes implemented in this PR? * `Pattern` has been moved from `Explanation` to `ConceptMap` * `Answer` interface is empty and requires further refactoring * Propagate `Pattern` throughout reasoner conclusions * Reconstruct `Explanation` objects from the query cache when the user requests an explanation a concept map * Update some tests * Solve previously unnoticed explanation bug: when re-querying the same query, the explanation is shallow (ie. a `LookupExplanation`) rather than the full reasoner tree
flyingsilverfin
added a commit
to typedb/typedb-driver
that referenced
this pull request
Nov 5, 2019
## What is the goal of this PR? Matching protocol changes in typedb/typedb-protocol#20, we now iteratively retrieve layers of the `Explanation` tree of a `ConceptMap` answer. `ConceptMap.explanation()` does a round trip to the Grakn server and retrieves a list of `ConceptMap` that explain how the given concept map was obtained. We also restrict `Explanation` to only be obtainable from `ConceptMap`, given that we have not implemented `Explanation` on delete, count, etc. queries. ## What are the changes implemented in this PR? * Implement typedb/typedb-protocol#20 * `ConceptMap` contains a `queryPattern` -- this is empty if reasoner is not triggered, and the new method `hasExplanation()` will return `false` * When `hasExplanation()` is true, it is possible to send the ConceptMap back to the server via `Transaction` to obtain an `Explanation` object * `Explanation` object only contains a list of `ConceptMap` * Refactor tests and add an Explanation test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the goal of this PR?
Explanations can exceed the gRPC message size limits. This change allows retrieving explanations separately from the
ConceptMapanswers themselves, allowing us to break the deep nesting and long messages contained in a single answer.The paradigm for retriving an explanation is now as follows:
A
ConceptMapalways contains apatternand a boolean flaghasExplanation. If theConceptMaphas an explanation, a call can be made using message typeExplanation.Req, returning a list ofConceptMapthat correspond to to a single layer of the explanation tree.Unused
Explanationmessages have been removed from answers other thanConceptMap.What are the changes implemented in this PR?
ConceptMapcontainsconceptMap,patternandhasExplanationExplanation.Req, returningExplanation.ResExplanationremoved from answers other thanConceptMap