Skip to content

Relationship tutorials bug - #76

Open
Charles-Johnson wants to merge 9 commits into
masterfrom
relationship-tutorials-bug
Open

Relationship tutorials bug#76
Charles-Johnson wants to merge 9 commits into
masterfrom
relationship-tutorials-bug

Conversation

@Charles-Johnson

@Charles-Johnson Charles-Johnson commented Apr 21, 2024

Copy link
Copy Markdown
Owner

Trying to demonstrate defining a sibling relationship but takes too long

@Charles-Johnson

Charles-Johnson commented Aug 23, 2026

Copy link
Copy Markdown
Owner Author

The latest commit takes slightly longer than the 15 second limit to process "let Alice is parent of Eve" but passes the tutorial tests if I raise the limit to 30 seconds. However, several lower level integration tests still fail:

  • existence_inference_rule:
thread 'context_search_test::existence_inference_rule::existence_inference_rule' panicked at zia/src/context_search_test/existence_inference_rule.rs:43:48:
called `Option::unwrap()` on a `None` value
  • inference_rule
thread 'context_search_test::inference_rule::inference_rule' panicked at zia/src/context_search_test/inference_rule.rs:88:55:
called `Option::unwrap()` on a `None` value
  • implied_reduction_via_implication_chain
thread 'context_search_test::implied_reduction_via_implication_chain::inference_rule' panicked at zia/src/context_search_test/implied_reduction_via_implication_chain.rs:133:14:
called `Option::unwrap()` on a `None` value
  • comparison_existence_implication_rule_test
thread 'context_search_test::comparison_existence_implication_rule::comparison_existence_implication_rule_test' panicked at zia/src/context_search_test/comparison_existence_implication_rule.rs:43:5:
assertion `left == right` failed
  left: (Incomparable, Reduction { reason: None, reversed_reason: None })
 right: (GreaterThan, Reduction { reason: Some(Rule { generalisation: GenericSyntaxTree ... (continued)
  • infered_precedence_test
thread 'context_test::infered_precedence::infered_precedence_test' panicked at zia/src/context_test/infered_precedence.rs:137:5:
assertion `left == right` failed
  left: "-> precedes let"
 right: "true"

These failures were introduced in bca493a

@Charles-Johnson

Copy link
Copy Markdown
Owner Author

I tried improving the performance by caching examples of "half generalisations" which are stored in the ContextSearch. The find_example_of_half_generalisation method was updated to write to this new cache after getting a result from the find_example method in the case of a cache miss.

I also added a method to ContextSearch called substitute_with_variable_mask_list which recurses through a SharedSyntax tree structure, using the to_ast method on nodes with an assigned concept ID. I didn't end up using it anywhere so need to remember what I intended this for.

git diff:

diff --git a/Cargo.lock b/Cargo.lock
index 2caead24..7d49511e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1457,6 +1457,7 @@ version = "0.1.0"
 dependencies = [
  "console_error_panic_hook",
  "seed",
+ "simple_logger",
  "wasm-bindgen-test",
  "web-sys",
  "zia",
diff --git a/zia-lang.org/crate/Cargo.toml b/zia-lang.org/crate/Cargo.toml
index a1b1419b..88b5aeed 100644
--- a/zia-lang.org/crate/Cargo.toml
+++ b/zia-lang.org/crate/Cargo.toml
@@ -14,6 +14,8 @@ crate-type = ["cdylib"]
 
 [dev-dependencies]
 wasm-bindgen-test = "=0.3.18"
+[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
+simple_logger = "1.15.0"
 
 [dependencies]
 zia = { path = "../../zia", version = "0.6.0"}
diff --git a/zia-lang.org/crate/src/page/home/tutorials.rs b/zia-lang.org/crate/src/page/home/tutorials.rs
index dba54aa3..e3741883 100644
--- a/zia-lang.org/crate/src/page/home/tutorials.rs
+++ b/zia-lang.org/crate/src/page/home/tutorials.rs
@@ -327,6 +327,8 @@ mod test {
     }
     #[test]
     fn relationships_tutorial() {
+        #[cfg(not(target_arch = "wasm32"))]
+        simple_logger::init().unwrap();
         let mut context = NEW_CONTEXT.clone();
         for step in TUTORIALS.1.steps {
             step.test(&mut context);
diff --git a/zia/src/concepts/mod.rs b/zia/src/concepts/mod.rs
index e63d2ed1..34dc974f 100755
--- a/zia/src/concepts/mod.rs
+++ b/zia/src/concepts/mod.rs
@@ -732,7 +732,7 @@ pub enum SpecificPart<Id: Eq + Hash> {
     String(String),
 }
 
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
 pub enum Hand {
     Left,
     Right,
diff --git a/zia/src/context_cache/macro.rs b/zia/src/context_cache/macro.rs
index 5f1a95ab..9b0fc9c6 100644
--- a/zia/src/context_cache/macro.rs
+++ b/zia/src/context_cache/macro.rs
@@ -212,7 +212,11 @@ impl<CI: ConceptId, SR: SharedReference> ReductionCacheList<CI, SR> {
                     .as_ref()
                     .map_or_else(|| String::from("None"), |r| r.0.to_string())
             );
-            self.head.insert(ast.key(), reduction_result.clone());
+            if ast.is_variable() || self.tail.is_none() {
+                self.head.insert(ast.key(), reduction_result.clone());
+            } else if let Some(tail) = &self.tail {
+                tail.insert_reduction(ast, reduction_result);
+            }
         } else {
             debug!(
                 "Skipping caching reduction: {} -> {}",
diff --git a/zia/src/context_search.rs b/zia/src/context_search.rs
index d0c502b5..850c42dc 100644
--- a/zia/src/context_search.rs
+++ b/zia/src/context_search.rs
@@ -29,7 +29,7 @@ use crate::{
     substitute::substitute,
     variable_mask_list::{VariableMask, VariableMaskList},
 };
-use dashmap::DashSet;
+use dashmap::{DashMap, DashSet};
 use log::debug;
 use maplit::{hashmap, hashset};
 use std::{
@@ -52,7 +52,13 @@ where
     bound_variable_syntax: &'v HashSet<SyntaxKey<CCI>>,
     phantom: PhantomData<SR::Share<DirectConceptDelta<CCI>>>,
     phantom2: PhantomData<CCI>,
+    cached_example_of_half_generalisation: DashMap<
+        HalfGeneralisationKey<CCI>,
+        Option<ExampleSubstitutions<CCI, SR>>,
+    >,
 }
+
+type HalfGeneralisationKey<CCI> = (SyntaxKey<CCI>, SyntaxKey<CCI>, CCI, Hand);
 impl<S, CCI: MixedConcept, SR: SharedReference> Debug
     for ContextSearch<'_, '_, S, CCI, SR>
 where
@@ -330,6 +336,27 @@ where
             })
     }
 
+    pub fn substitute_with_variable_mask_list(
+        &self,
+        ast: &SharedSyntax<CCI, SR>,
+    ) -> SharedSyntax<CCI, SR> {
+        ast.get_concept().map_or_else(
+            || {
+                ast.get_expansion().map_or_else(
+                    || ast.clone(),
+                    |(l, r)| {
+                        self.contract_pair(
+                            &self.substitute_with_variable_mask_list(&l),
+                            &self.substitute_with_variable_mask_list(&r),
+                        )
+                        .share()
+                    },
+                )
+            },
+            |c| self.to_ast(&c),
+        )
+    }
+
     /// Returns the abstract syntax from two syntax parts, using the label and concept of the composition of associated concepts if it exists.
     pub fn contract_pair(
         &self,
@@ -438,7 +465,6 @@ where
         generalisation: &SharedSyntax<CCI, SR>,
         truths: impl Iterator<Item = S::ConceptId>,
     ) -> Option<ExampleSubstitutions<CCI, SR>> {
-        debug!("find_example({})", generalisation.as_ref());
         self.find_examples(generalisation.clone(), truths).next()
     }
 
@@ -574,7 +600,6 @@ where
         generalisation: SharedSyntax<CCI, SR>,
         equivalence_set: impl Iterator<Item = S::ConceptId> + 'a, /* All concepts that are equal to generalisation */
     ) -> impl Iterator<Item = ExampleSubstitutions<CCI, SR>> + 'a {
-        debug!("find_examples({})", generalisation.as_ref());
         let iterator: Box<dyn Iterator<Item = ExampleSubstitutions<CCI, SR>>>;
         if let Some((left, right)) = generalisation.get_expansion() {
             iterator = Box::new(self.find_examples_of_branched_generalisation(
@@ -742,7 +767,6 @@ where
         mut equivalence_set_of_composition: impl Iterator<Item = S::ConceptId> + 'a,
         non_generalised_hand: Hand,
     ) -> Option<ExampleSubstitutions<CCI, SR>> {
-        debug!("find_examples_of_half_generalisation({}, {}, {non_generalised_hand:?})", generalised_part.as_ref(), non_generalised_part.as_ref());
         // TODO try to test if this needs to be a flat_map call
         equivalence_set_of_composition.find_map(move |equivalent_concept_id| {
             self.find_example_of_half_generalisation(
@@ -776,51 +800,68 @@ where
         non_generalised_hand: Hand,
         or_else: impl FnOnce(S::ConceptId) -> Option<ExampleSubstitutions<CCI, SR>>,
     ) -> Option<ExampleSubstitutions<CCI, SR>> {
-        // TODO cache this calculation
+        debug!("find_example_of_half_generalisation({}, {}, {equivalent_concept_id}, {non_generalised_hand:?})", generalised_part_clone.as_ref(), non_generalised_part.as_ref());
+        let key = (
+            generalised_part_clone.key(),
+            non_generalised_part.key(),
+            equivalent_concept_id,
+            non_generalised_hand,
+        );
         let equivalent_concept = self
             .snap_shot
             .read_concept(self.delta.as_ref(), equivalent_concept_id);
         let (left, right) = equivalent_concept.get_composition()?;
-        let (equivalent_non_generalised_hand, equivalent_generalised_hand) =
-            match non_generalised_hand {
-                Hand::Left => (left, right),
-                Hand::Right => (right, left),
-            };
-        if Some(equivalent_non_generalised_hand)
-            != non_generalised_part.get_concept()
-        {
-            if self
-                .snap_shot
-                .read_concept(
-                    self.delta.as_ref(),
-                    equivalent_non_generalised_hand,
-                )
-                .free_variable()
+        let non_generalised_id = non_generalised_part.get_concept()?;
+        let example_hand = match non_generalised_hand {
+            Hand::Left => (left == non_generalised_id).then_some(right)?,
+            Hand::Right => (right == non_generalised_id).then_some(left)?,
+        };
+        let compute = || {
+            let (equivalent_non_generalised_hand, equivalent_generalised_hand) =
+                match non_generalised_hand {
+                    Hand::Left => (left, right),
+                    Hand::Right => (right, left),
+                };
+            if Some(equivalent_non_generalised_hand)
+                != non_generalised_part.get_concept()
             {
-                return self.find_example(generalised_part_clone, iter::once(equivalent_generalised_hand)).and_then(|subs| {
+                if self
+                    .snap_shot
+                    .read_concept(
+                        self.delta.as_ref(),
+                        equivalent_non_generalised_hand,
+                    )
+                    .free_variable()
+                {
+                    return self.find_example(generalised_part_clone, iter::once(equivalent_generalised_hand)).and_then(|subs| {
                         // Could have a more efficient method for this
                         subs.consistent_merge(ExampleSubstitutions{example: hashmap!{equivalent_non_generalised_hand => non_generalised_part.clone()}, ..Default::default()})
                     });
+                }
+                return None;
             }
-            return None;
-        }
-        self.find_example(
-            generalised_part_clone,
-            iter::once(equivalent_generalised_hand),
-        )
-        .or_else(|| {
-            let non_generalised_id = non_generalised_part.get_concept()?;
-            let example_hand = match non_generalised_hand {
-                Hand::Left => (left == non_generalised_id).then_some(right)?,
-                Hand::Right => (right == non_generalised_id).then_some(left)?,
-            };
-            let example_hand_syntax = self.to_ast(&example_hand);
-            GenericSyntaxTree::<CCI, SR>::check_example(
-                &example_hand_syntax,
-                generalised_part_clone,
-            )
-            .or_else(|| or_else(example_hand))
-        })
+            let result = self
+                .find_example(
+                    generalised_part_clone,
+                    iter::once(equivalent_generalised_hand),
+                )
+                .or_else(|| {
+                    let example_hand_syntax = self.to_ast(&example_hand);
+                    GenericSyntaxTree::<CCI, SR>::check_example(
+                        &example_hand_syntax,
+                        generalised_part_clone,
+                    )
+                });
+            self.cached_example_of_half_generalisation
+                .insert(key.clone(), result.clone());
+            result
+        };
+
+        let result = self
+            .cached_example_of_half_generalisation
+            .get(&key)
+            .map_or_else(compute, |result| result.clone());
+        result.or_else(|| or_else(example_hand))
     }
 
     // Reduces a syntax tree based on the properties of the left branch and the branches of the right branch
@@ -1132,6 +1173,7 @@ where
         delta: SR::Share<NestedDelta<CCI, SR>>,
     ) -> Self {
         ContextSearch::<'s, 'v> {
+            cached_example_of_half_generalisation: DashMap::new(),
             concept_inferring: self.concept_inferring.clone(),
             bound_variable_syntax: self.bound_variable_syntax,
             caches: self.caches.spawn(cache),
@@ -1484,6 +1526,7 @@ where
         }: ContextReferences<'c, 's, 'v, S, SR, CCI>,
     ) -> Self {
         Self {
+            cached_example_of_half_generalisation: DashMap::default(),
             concept_inferring: HashSet::default(),
             bound_variable_syntax,
             snap_shot,
diff --git a/zia/src/variable_mask_list.rs b/zia/src/variable_mask_list.rs
index 47adc19a..61b88592 100644
--- a/zia/src/variable_mask_list.rs
+++ b/zia/src/variable_mask_list.rs
@@ -8,7 +8,7 @@ use crate::{
 
 #[derive(Clone)]
 pub struct VariableMaskList<CI: ConceptId, SR: SharedReference> {
-    head: VariableMask<CI, SR>,
+    pub head: VariableMask<CI, SR>,
     tail: Option<SR::Share<Self>>,
 }

@Charles-Johnson

Copy link
Copy Markdown
Owner Author

I'll see if I can merge any of these commits separately into master without breaking the tests

@Charles-Johnson

Copy link
Copy Markdown
Owner Author

After cherry-picking commits up until 08f4af2, I found that the array_tutorial test was flaky with the following failure:

thread 'page::home::tutorials::test::array_tutorial' panicked at zia-lang.org/crate/src/page/home/tutorials.rs:320:9:
assertion `left == right` failed: Failed at ([ 5 , 3 ])[ 1 ]
  left: "([ 5 , 3 ]) [ (0 +1) ]"
 right: "3"

@Charles-Johnson

Copy link
Copy Markdown
Owner Author

Eve is a sibling of Bob fails to reduce on the master branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant