typedb icon indicating copy to clipboard operation
typedb copied to clipboard

Non-inferred concepts are incorrectly marked as explainable

Open james-whiteside opened this issue 2 years ago • 4 comments

Description

Non-inferred concepts are incorrectly marked as explainable by conceptMap.explainables(). Attempting to explain those concepts with transaction.query().explain() returns an empty result. This is easily worked around, but can cause pretty detrimental unexpected behaviour if the user is not aware.

Environment

  1. MacOS 12.6.1
  2. TypeDB 2.14.3
  3. Client Python.

Reproducible Steps

Steps to create the smallest reproducible scenario:

  1. Insert some data and add relations via rule inferrence.
  2. Execute a query using the reasoner and check for concepts that appear in conceptMap.explainables() but have concept.isInferred == false.
  3. Instances were also observed where there were concepts in conceptMap.explainables() despite no inference having occured in the query at all. It is unclear if this is dependent on inferred concepts existing in the database.

Expected Output

The number of concepts in conceptMap.explainables() should equal the number of concepts with concept.isInferred == true.

Actual Output

There are more concepts in conceptMap.explainables() than concepts with concept.isInferred == true.

Additional information

Studio has a workaround built in to prevent nodes being incorrectly marked as explainable: https://github.com/vaticle/typedb-studio/blob/519386e76ebfaa31d6dce7f69a77666e1ebc44c7/framework/graph/GraphBuilder.kt#L78

james-whiteside avatar Feb 03 '23 12:02 james-whiteside

The following issue is likely caused by this bug when working through the IAM demo:

  1. Download demo.
  2. Run define-schema.tql and insert-data.tql.
  3. Run in order Examples 3, 5, and 6.
  4. Run Example 7 with explanations on and attempt to explain the inferred validity.
  5. Studio throws [SYS01] TypeDB Studio System: Unexpected error occurred with a coroutine: Not explainable

james-whiteside avatar Jun 22 '23 10:06 james-whiteside

To add the side of explanations to the problem, so we don't miss any spots when we're refactoring explainable to be more intuitive:

Currently, explainability is a property of a constraint in the query, not of "a concept". Put simply, a constraint is explainable if it can be satisfied by a rule. Consequently, If an explainable constraint is satisfied by a relation or ownership that already exists, it's still treated as explainable, just with no explanations apart from "Is in the database". This is useful in 2 ways:

  1. The set of explainables remains constant across all the answers to a query.
  2. If we support querying multiple explanations, we could provide alternate actual explanations in addition to "Is in the database".

krishnangovindraj avatar Jun 23 '23 10:06 krishnangovindraj

It is worth noting that returning explanations for a fact that is written to disk but could also have been inferred is a feature that has been requested.

james-whiteside avatar Jun 23 '23 10:06 james-whiteside

Following discussion with @krishnangovindraj and @flyingsilverfin, the following UX requirements are set out for the triggering of explanations as a long-term goal:

  1. All inferred constraints (concepts/edges) shall be explainable.
  2. All inferred constraints shall have at least one explanation.
  3. The set of explanations for an inferred constraint shall be the same regardless of the original query issued.
  4. The set of explanations for an inferred constraint shall contain every inference-based explanation for that constraint permitted by the entire schema and data.
  5. The set of explanations for an inferred constraint may contain the fact that it is written to disk in the case that it is both written and also inferrable.
  6. All non-inferred constraints may be explainable, in which case the only explanation shall be that they are written to disk.

These requirements relate only to the triggering of explanations, with the requirements for the response format to be separately defined.

james-whiteside avatar Jun 23 '23 11:06 james-whiteside