diff options
author | JSDurand <mmemmew@gmail.com> | 2023-06-02 15:00:48 +0800 |
---|---|---|
committer | JSDurand <mmemmew@gmail.com> | 2023-06-02 15:00:48 +0800 |
commit | 8486474f377faf2d800d79166a7abe6b975e3e50 (patch) | |
tree | f06baa22bafebf9393869664067e9cf308c92634 /chain/src/default.rs | |
parent | 1455da10f943e2aa1bdf26fb2697dafccc61e073 (diff) |
Fix a bug of duplication from planting after sploing
I should have staged and committed these changes separately, but I am
too lazy to deal with that.
The main changes in this commit are that I added the derive macro that
automates the delegation of the Graph trait. This saves a lot of
boiler-plate codes.
The second main change, perhaps the most important one, is that I
found and tried to fix a bug that caused duplication of nodes. The
bug arises from splitting or cloning a node multiple times, and
immediately planting the same fragment under the new "sploned" node.
That is, when we try to splone the node again, we found that we need
to splone, because the node that was created by the same sploning
process now has a different label because of the planting of the
fragment. Then after the sploning, we plant the fragment again. This
makes the newly sploned node have the same label (except for the clone
index) and the same children as the node that was sploned and planted
in the previous rounds.
The fix is to check for the existence of a node that has the same set
of children as the about-to-be-sploned node, except for the last one,
which contains the about-to-be-planted fragment as a prefix. If that
is the case, treat it as an already existing node, so that we do not
have to splone the node again.
This is consistent with the principle to not create what we do not
need.
Diffstat (limited to 'chain/src/default.rs')
-rw-r--r-- | chain/src/default.rs | 122 |
1 files changed, 20 insertions, 102 deletions
diff --git a/chain/src/default.rs b/chain/src/default.rs index 1df68b5..e61df41 100644 --- a/chain/src/default.rs +++ b/chain/src/default.rs @@ -17,6 +17,8 @@ use graph::{ labelled::DLGBuilder, Builder, DLGraph, Graph, LabelExtGraph, LabelGraph, ParentsGraph, }; +use graph_macro::Graph; + use std::collections::{HashMap as Map, HashSet, TryReserveError}; /// The errors related to taking derivatives by chain rule. @@ -204,6 +206,8 @@ impl Iterator for DerIter { // we just query again with the new bottom and top. This means we do // not need to clear the map. +// REVIEW: Why is the above needed? + /// This represents a tuple of bottom and top forest positions. /// /// It is supposed to be used as keys to query the reduction @@ -241,12 +245,13 @@ impl BoTop { /// The value records a set of tuples of the form `(non-terminal, /// starting_position)`. Such a tuple means we want to reduce from /// the expansion of the given `non-terminal`, which expands from the -/// given starting position. We need to restrict the +/// given starting position. We need to restrict the pub(crate) type Reducer = Map<(usize, usize), HashSet<usize>>; /// A default implementation for the [`Chain`] trait. -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone, Default, Graph)] pub struct DefaultChain { + #[graph] graph: DLGraph<Edge>, atom: DefaultAtom, current: usize, @@ -292,51 +297,6 @@ impl DefaultChain { } } -impl Graph for DefaultChain { - type Iter<'a> = <DLGraph<Edge> as Graph>::Iter<'a> - where - Self: 'a; - - #[inline] - fn is_empty(&self) -> bool { - self.graph.is_empty() - } - - #[inline] - fn nodes_len(&self) -> usize { - self.graph.nodes_len() - } - - #[inline] - fn edges_len(&self) -> Option<usize> { - self.graph.edges_len() - } - - #[inline] - fn children_of(&self, node_id: usize) -> Result<Self::Iter<'_>, GError> { - self.graph.children_of(node_id) - } - - #[inline] - fn degree(&self, node_id: usize) -> Result<usize, GError> { - self.graph.degree(node_id) - } - - #[inline] - fn is_empty_node(&self, node_id: usize) -> Result<bool, GError> { - self.graph.is_empty_node(node_id) - } - - #[inline] - fn has_edge(&self, source: usize, target: usize) -> Result<bool, GError> { - self.graph.has_edge(source, target) - } - - fn replace_by_builder(&mut self, _builder: impl graph::Builder<Result = Self>) { - unimplemented!("I shall refactor this") - } -} - impl LabelGraph<Edge> for DefaultChain { type Iter<'a> = <DLGraph<Edge> as LabelGraph<Edge>>::Iter<'a> where @@ -689,49 +649,24 @@ impl Chain for DefaultChain { let atom_moved = atom_label.get_moved(); - if pos == 4 { - dbg!(atom_label); - } - match *atom_label.get_value() { Some(Ter(ter)) if ter == t => { - let new_pavi: PaVi; - - if !no_item { + let new_pavi = if !no_item { // prepare forest fragment let fragment = generate_fragment([atom_moved.into(), Ter(ter).into()], pos)?; - if pos == 4 { - dbg!(atom_moved, label); - self.forest - .print_viz(&format!( - "pos4tb - {atom_moved}-{:?}.gv", - label.true_source() - )) - .unwrap(); - } - - new_pavi = self.forest.insert_item( + self.forest.insert_item( &self.reducer, *label, fragment, atom_child_iter.clone(), &self.atom, - )?; - - if pos == 4 { - self.forest - .print_viz(&format!( - "pos4ta - {atom_moved}-{:?}.gv", - label.true_source() - )) - .unwrap(); - } + )? } else { - new_pavi = PaVi::default(); - } + PaVi::default() + }; let generator_input = ( &self.graph, @@ -781,13 +716,6 @@ impl Chain for DefaultChain { let first_fragment = generate_fragment([atom_moved.into(), Non(non).into()], pos)?; - if pos == 4 { - dbg!(atom_moved, label); - self.forest - .print_viz(&format!("pos4nb - {atom_moved}-{:?}.gv", label)) - .unwrap(); - } - first_segment_pavi = self.forest.insert_item( &self.reducer, *label, @@ -796,12 +724,6 @@ impl Chain for DefaultChain { &self.atom, )?; - if pos == 4 { - self.forest - .print_viz(&format!("pos4na - {atom_moved}-{:?}.gv", label)) - .unwrap(); - } - let virtual_fragment = DefaultForest::new_leaf(GrammarLabel::new(Ter(t), pos)); @@ -816,12 +738,6 @@ impl Chain for DefaultChain { std::iter::empty(), &self.atom, )?; - - if pos == 4 { - self.forest - .print_viz(&format!("pos4va - {atom_moved}-{:?}.gv", label)) - .unwrap(); - } } else { first_segment_pavi = PaVi::default(); virtual_pavi = PaVi::default(); @@ -958,7 +874,7 @@ impl Chain for DefaultChain { dbg!(&self.accepting_sources); - self.forest.print_viz("dbg forest before.gv").unwrap(); + // self.forest.print_viz("dbg forest before.gv").unwrap(); for pavi in self.accepting_sources.iter() { match pavi { @@ -997,9 +913,9 @@ impl Chain for DefaultChain { let mut seen_nodes: HashSet<usize> = HashSet::with_capacity(stack.len()); - dbg!(&stack); + // dbg!(&stack); - self.forest.print_viz("dbg forest.gv").unwrap(); + // self.forest.print_viz("dbg forest.gv").unwrap(); while let Some(mut top) = stack.pop() { if seen_nodes.contains(&top) { @@ -1251,10 +1167,12 @@ mod test_chain { chain.forest.print_viz("forest3.gv")?; chain.chain(1, 4, no_item)?; chain.forest.print_viz("forest4.gv")?; - chain.end_of_input()?; - chain.forest.print_viz("forest.gv")?; - chain.forest.print_closed_viz("closed.gv")?; + if !no_item { + chain.end_of_input()?; + chain.forest.print_viz("forest.gv")?; + chain.forest.print_closed_viz("closed.gv")?; + } chain.graph.print_viz("chain.gv")?; chain.atom.print_nfa("nfa.gv")?; item::default::print_labels(&chain.atom, &chain.forest)?; |