Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Compute affected nodes for SimpleReplacement #600

Merged
merged 3 commits into from
Oct 11, 2023
Merged

feat: Compute affected nodes for SimpleReplacement #600

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cdcc733
feat: Compute affected nodes for `SimpleReplacement`
aborgna-q Oct 11, 2023
a34662f
drive-by: add some inlines
aborgna-q Oct 11, 2023
ffeb85e
Reword docs, s/affected_nodes/invalidation_set/
aborgna-q Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion src/hugr/rewrite/simple_replace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub struct SimpleReplacement {

impl SimpleReplacement {
/// Create a new [`SimpleReplacement`] specification.
#[inline]
pub fn new(
subgraph: SiblingSubgraph,
replacement: Hugr,
Expand All @@ -45,14 +46,30 @@ impl SimpleReplacement {
}

/// The replacement hugr.
#[inline]
pub fn replacement(&self) -> &Hugr {
&self.replacement
}

/// Subgraph to be replaced.
#[inline]
pub fn subgraph(&self) -> &SiblingSubgraph {
&self.subgraph
}

/// Returns the nodes affected by the replacement.
///
/// This includes the nodes in the subgraph and the boundary neighbours that
/// are referenced by the replacement.
///
/// Two `SimpleReplacement`s can be composed if their affected nodes are
/// disjoint.
#[inline]
pub fn affected_nodes(&self) -> impl Iterator<Item = Node> + '_ {
let subcirc = self.subgraph.nodes().iter().copied();
let out_neighs = self.nu_out.keys().map(|&(n, _)| n);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only picks up the output neighbours, I think? Do we want to include the input neighbours?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The input nodes referenced in the rewrite are part of the removed set.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(so modifying the input neighbours doesn't invalidate the rewrite)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I see what you mean, though I think maybe the documentation needs clarifying, or the function renaming? E.g. in the test case, intuitively first two H gates in the circuit (preceding the replaced subcircuit) would count as "affected by" the replacement just as much as the succeeding output node.

Hmm, "referenced nodes"? Not convinced. Anyway no quibbles with the implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe invalidation_set?

subcirc.chain(out_neighs)
}
}

impl Rewrite for SimpleReplacement {
Expand Down Expand Up @@ -203,7 +220,7 @@ pub(in crate::hugr::rewrite) mod test {
use itertools::Itertools;
use portgraph::Direction;
use rstest::{fixture, rstest};
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};

use crate::builder::{
BuildError, Container, DFGBuilder, Dataflow, DataflowHugr, DataflowSubContainer,
Expand Down Expand Up @@ -384,6 +401,11 @@ pub(in crate::hugr::rewrite) mod test {
nu_inp,
nu_out,
};
assert_eq!(
HashSet::<_>::from_iter(r.affected_nodes()),
HashSet::<_>::from_iter([h_node_cx, h_node_h0, h_node_h1, h_outp_node]),
);

h.apply_rewrite(r).unwrap();
// Expect [DFG] to be replaced with:
// ┌───┐┌───┐
Expand Down