Add tests and only add the inherited edges for the invalid node itself.

thomaskrause · thomaskrause · commit b249354018fe · 2024-08-20T16:50:18.000+02:00
All covered nodes of an invalidated node will also be invalidated, so we don't need to implicitly add the edges when collected the covered token of the invalid node.
diff --git a/graphannis/src/annis/db/aql/model.rs b/graphannis/src/annis/db/aql/model.rs
@@ -126,47 +126,53 @@ fn calculate_inherited_coverage_edges(
         .collect();
     let combined_gs = UnionEdgeContainer::new(all_text_coverage_gs);
 
-    // Remember the non-token and token nodes, so we can connect all non-token
-    // nodes to the covered token.
-    let mut indirectly_covered_nodes = FxHashSet::default();
-    indirectly_covered_nodes.insert(n);
     let mut covered_token = FxHashSet::default();
 
-    let tok_helper = TokenHelper::new(graph)?;
-
-    for step in CycleSafeDFS::new(&combined_gs, n, 1, usize::MAX) {
-        let step = step?;
-        if tok_helper.is_token(step.node)? {
-            covered_token.insert(step.node);
-        } else {
-            indirectly_covered_nodes.insert(step.node);
-        }
-    }
-
-    let coverage_gs = tok_helper.get_gs_coverage().clone();
-
-    if let Ok(gs_cov) = graph.get_or_create_writable(&AnnotationComponent::new(
+    let inherited_cov_component = AnnotationComponent::new(
         AnnotationComponentType::Coverage,
         ANNIS_NS.into(),
         "inherited-coverage".into(),
-    )) {
-        // Connect all non-token nodes to the covered token nodes if no such direct coverage already exists
-        for source in indirectly_covered_nodes {
-            for target in &covered_token {
-                let mut needs_edge = true;
-                for gs in coverage_gs.iter() {
-                    if gs.is_connected(source, *target, 1, std::ops::Bound::Included(1))? {
-                        needs_edge = false;
-                        break;
-                    }
-                }
+    );
 
-                if needs_edge {
-                    gs_cov.add_edge(Edge {
-                        source,
-                        target: *target,
-                    })?;
-                }
+    {
+        let tok_helper = TokenHelper::new(graph)?;
+        for step in CycleSafeDFS::new(&combined_gs, n, 1, usize::MAX) {
+            let step = step?;
+            if tok_helper.is_token(step.node)? {
+                covered_token.insert(step.node);
+            }
+        }
+    };
+    let other_coverage_gs: Vec<Arc<dyn GraphStorage>> = graph
+        .get_all_components(Some(AnnotationComponentType::Coverage), None)
+        .into_iter()
+        .filter(|c| c != &inherited_cov_component)
+        .filter_map(|c| graph.get_graphstorage(&c))
+        .filter(|gs| {
+            if let Some(stats) = gs.get_statistics() {
+                stats.nodes > 0
+            } else {
+                true
+            }
+        })
+        .collect();
+
+    // Connect all non-token nodes to the covered token nodes if no such direct coverage already exists
+    let mut direct_coverage_targets = FxHashSet::default();
+    for gs in other_coverage_gs.iter() {
+        for target in gs.get_outgoing_edges(n) {
+            direct_coverage_targets.insert(target?);
+        }
+    }
+    let gs_cov = graph.get_or_create_writable(&inherited_cov_component)?;
+
+    for target in &covered_token {
+        if n != *target {
+            if !direct_coverage_targets.contains(target) {
+                gs_cov.add_edge(Edge {
+                    source: n,
+                    target: *target,
+                })?;
             }
         }
     }
@@ -283,7 +289,7 @@ impl AQLUpdateGraphIndex {
         let all_cov_components =
             graph.get_all_components(Some(AnnotationComponentType::Coverage), None);
         let all_dom_components =
-            graph.get_all_components(Some(AnnotationComponentType::Dominance), Some(""));
+            graph.get_all_components(Some(AnnotationComponentType::Dominance), None);
 
         // go over each node and calculate the left-most and right-most token
         for invalid in self.invalid_nodes.iter()? {
diff --git a/graphannis/src/annis/db/aql/model/tests.rs b/graphannis/src/annis/db/aql/model/tests.rs
@@ -1,7 +1,19 @@
 use std::{fs::File, path::PathBuf};
 
-use crate::{annis::db::aql::model::CorpusSize, AnnotationGraph};
+use crate::{
+    annis::db::{aql::model::CorpusSize, example_generator},
+    model::AnnotationComponent,
+    AnnotationGraph,
+};
 use assert_matches::assert_matches;
+use graphannis_core::graph::{
+    storage::GraphStorage,
+    update::{GraphUpdate, UpdateEvent},
+    NODE_NAME_KEY,
+};
+use itertools::Itertools;
+
+use super::AnnotationComponentType::Coverage;
 
 #[test]
 fn global_stats_token_count() {
@@ -21,3 +33,244 @@ fn global_stats_token_count() {
             && *segmentation_count.get("diplomatic").unwrap() == 11 
             && *segmentation_count.get("norm").unwrap() == 13);
 }
+
+#[test]
+fn inherited_cov_edges_simple_tokenization() {
+    // Ad a simple dominance node structure above the example sentence.
+    let mut u = GraphUpdate::new();
+    example_generator::create_corpus_structure_simple(&mut u);
+    example_generator::create_tokens(&mut u, Some("root/doc1"), Some("root/doc1"));
+    example_generator::make_span(
+        &mut u,
+        "root/doc1#span1",
+        &["root/doc1#tok1", "root/doc1#tok2", "root/doc1#tok3"],
+        true,
+    );
+    example_generator::make_span(
+        &mut u,
+        "root/doc1#span2",
+        &["root/doc1#tok4", "root/doc1#tok5"],
+        true,
+    );
+    u.add_event(UpdateEvent::AddNode {
+        node_name: "root/doc1#struct1".to_string(),
+        node_type: "node".to_string(),
+    })
+    .unwrap();
+    u.add_event(UpdateEvent::AddNodeLabel {
+        node_name: "root/doc1#struct1".to_string(),
+        anno_ns: "test".to_string(),
+        anno_name: "cat".to_string(),
+        anno_value: "P".to_string(),
+    })
+    .unwrap();
+    u.add_event(UpdateEvent::AddEdge {
+        source_node: "root/doc1#struct1".to_string(),
+        target_node: "root/doc1#span1".to_string(),
+        layer: "test".to_string(),
+        component_type: "Dominance".to_string(),
+        component_name: "edge".to_string(),
+    })
+    .unwrap();
+    u.add_event(UpdateEvent::AddEdge {
+        source_node: "root/doc1#struct1".to_string(),
+        target_node: "root/doc1#span2".to_string(),
+        layer: "test".to_string(),
+        component_type: "Dominance".to_string(),
+        component_name: "edge".to_string(),
+    })
+    .unwrap();
+    u.add_event(UpdateEvent::AddNode {
+        node_name: "root/doc1#struct2".to_string(),
+        node_type: "node".to_string(),
+    })
+    .unwrap();
+    u.add_event(UpdateEvent::AddNodeLabel {
+        node_name: "root/doc1#struct2".to_string(),
+        anno_ns: "test".to_string(),
+        anno_name: "cat".to_string(),
+        anno_value: "ROOT".to_string(),
+    })
+    .unwrap();
+    u.add_event(UpdateEvent::AddEdge {
+        source_node: "root/doc1#struct2".to_string(),
+        target_node: "root/doc1#struct1".to_string(),
+        layer: "test".to_string(),
+        component_type: "Dominance".to_string(),
+        component_name: "edge".to_string(),
+    })
+    .unwrap();
+
+    let mut g = AnnotationGraph::with_default_graphstorages(false).unwrap();
+    g.apply_update(&mut u, |_| {}).unwrap();
+
+    // Check that the inherited coverage edges have been created
+    let gs = g
+        .get_graphstorage_as_ref(&AnnotationComponent::new(
+            Coverage,
+            "annis".into(),
+            "inherited-coverage".into(),
+        ))
+        .unwrap();
+    let sources: Vec<_> = gs
+        .source_nodes()
+        .map(|n| {
+            g.get_node_annos()
+                .get_value_for_item(&n.unwrap(), &NODE_NAME_KEY)
+                .unwrap()
+                .unwrap()
+                .to_string()
+        })
+        .sorted()
+        .collect();
+    assert_eq!(sources, vec!["root/doc1#struct1", "root/doc1#struct2"]);
+
+    // Also check that the edges target the right token
+    assert_out_edges(
+        &g,
+        gs,
+        "root/doc1#struct1",
+        &[
+            "root/doc1#tok1",
+            "root/doc1#tok2",
+            "root/doc1#tok3",
+            "root/doc1#tok4",
+            "root/doc1#tok5",
+        ],
+    );
+    assert_out_edges(
+        &g,
+        gs,
+        "root/doc1#struct2",
+        &[
+            "root/doc1#tok1",
+            "root/doc1#tok2",
+            "root/doc1#tok3",
+            "root/doc1#tok4",
+            "root/doc1#tok5",
+        ],
+    );
+}
+
+#[test]
+fn inherited_cov_edges_multiple_segmentation() {
+    let mut u = GraphUpdate::new();
+    example_generator::create_corpus_structure_simple(&mut u);
+    example_generator::create_multiple_segmentations(&mut u, "root/doc1");
+    // Add a simple dominance node structure above the "a" segmentation
+    example_generator::make_span(
+        &mut u,
+        "root/doc1#span1",
+        &["root/doc1#a1", "root/doc1#a2", "root/doc1#a3"],
+        true,
+    );
+    example_generator::make_span(&mut u, "root/doc1#span2", &["root/doc1#a4"], true);
+    u.add_event(UpdateEvent::AddNode {
+        node_name: "root/doc1#struct1".to_string(),
+        node_type: "node".to_string(),
+    })
+    .unwrap();
+    u.add_event(UpdateEvent::AddNodeLabel {
+        node_name: "root/doc1#struct1".to_string(),
+        anno_ns: "test".to_string(),
+        anno_name: "cat".to_string(),
+        anno_value: "ROOT".to_string(),
+    })
+    .unwrap();
+    u.add_event(UpdateEvent::AddEdge {
+        source_node: "root/doc1#struct1".to_string(),
+        target_node: "root/doc1#span1".to_string(),
+        layer: "test".to_string(),
+        component_type: "Dominance".to_string(),
+        component_name: "edge".to_string(),
+    })
+    .unwrap();
+    u.add_event(UpdateEvent::AddEdge {
+        source_node: "root/doc1#struct1".to_string(),
+        target_node: "root/doc1#span2".to_string(),
+        layer: "test".to_string(),
+        component_type: "Dominance".to_string(),
+        component_name: "edge".to_string(),
+    })
+    .unwrap();
+
+    let mut g = AnnotationGraph::with_default_graphstorages(false).unwrap();
+    g.apply_update(&mut u, |_| {}).unwrap();
+
+    // TODO Check that the inherited coverage edges have been created
+    let gs = g
+        .get_graphstorage_as_ref(&AnnotationComponent::new(
+            Coverage,
+            "annis".into(),
+            "inherited-coverage".into(),
+        ))
+        .unwrap();
+
+    let sources: Vec<_> = gs
+        .source_nodes()
+        .map(|n| {
+            g.get_node_annos()
+                .get_value_for_item(&n.unwrap(), &NODE_NAME_KEY)
+                .unwrap()
+                .unwrap()
+                .to_string()
+        })
+        .sorted()
+        .collect();
+    assert_eq!(
+        sources,
+        vec!["root/doc1#span1", "root/doc1#span2", "root/doc1#struct1"]
+    );
+
+    // Also check that the edges target the right timeline items (and not the segmentation nodes)
+    assert_out_edges(
+        &g,
+        gs,
+        "root/doc1#span1",
+        &[
+            "root/doc1#tli1",
+            "root/doc1#tli2",
+            "root/doc1#tli3",
+            "root/doc1#tli4",
+        ],
+    );
+    assert_out_edges(&g, gs, "root/doc1#span2", &["root/doc1#tli5"]);
+    assert_out_edges(
+        &g,
+        gs,
+        "root/doc1#struct1",
+        &[
+            "root/doc1#tli1",
+            "root/doc1#tli2",
+            "root/doc1#tli3",
+            "root/doc1#tli4",
+            "root/doc1#tli5",
+        ],
+    );
+}
+
+fn assert_out_edges(
+    graph: &AnnotationGraph,
+    gs: &dyn GraphStorage,
+    source: &str,
+    expected: &[&str],
+) {
+    let out: Vec<_> = gs
+        .get_outgoing_edges(
+            graph
+                .get_node_annos()
+                .get_node_id_from_name(source)
+                .unwrap()
+                .unwrap(),
+        )
+        .map(|t| {
+            graph
+                .get_node_annos()
+                .get_value_for_item(&t.unwrap(), &NODE_NAME_KEY)
+                .unwrap()
+                .unwrap()
+                .to_string()
+        })
+        .collect();
+    assert_eq!(out, expected);
+}
diff --git a/graphannis/src/annis/db/corpusstorage/tests.rs b/graphannis/src/annis/db/corpusstorage/tests.rs
@@ -676,7 +676,7 @@ fn subgraph_with_segmentation() {
     ];
     for (i, t) in seg_tokens.iter().enumerate() {
         let node_name = format!("root/doc1#seg{}", i);
-        example_generator::create_token_node(&mut g, &node_name, t, Some("root/doc1"));
+        example_generator::create_token_node(&mut g, &node_name, t, None, None, Some("root/doc1"));
         g.add_event(UpdateEvent::AddNodeLabel {
             node_name,
             anno_ns: "default_ns".to_string(),
@@ -1488,6 +1488,8 @@ fn reoptimize_corpussizeconfig() {
         &mut u,
         "rootCorpus/subCorpus1/doc1#sTok12",
         "!",
+        None,
+        None,
         Some("rootCorpus/subCorpus1/doc1#sText1"),
     );
     u.add_event(UpdateEvent::AddEdge {
diff --git a/graphannis/src/annis/db/example_generator.rs b/graphannis/src/annis/db/example_generator.rs
