Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 19 additions & 14 deletions tket/src/circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,10 @@ lazy_static! {
set
};
}
/// The [IGNORED_EXTENSION_OPS] definition depends on the buggy behaviour of [`NamedOp::name`], which returns bare names instead of scoped names on some cases.
/// Once this test starts failing it should be time to drop the `format!("prelude.{}", ...)`.
/// https://github.com/CQCL/hugr/issues/1496
/// The [IGNORED_EXTENSION_OPS] definition depends on the buggy behaviour of
/// [`NamedOp::name`], which returns bare names instead of scoped names on some
/// cases. Once this test starts failing it should be time to drop the
/// `format!("prelude.{}", ...)`. https://github.com/CQCL/hugr/issues/1496
#[test]
fn issue_1496_remains() {
assert_eq!("Noop", NoopDef.opdef_id())
Expand Down Expand Up @@ -134,8 +135,8 @@ impl<T: HugrView> Circuit<T> {
/// If the circuit is a function definition, returns the name of the
/// function.
///
/// If the name is empty or the circuit is not a function definition, returns
/// `None`.
/// If the name is empty or the circuit is not a function definition,
/// returns `None`.
#[inline]
pub fn name(&self) -> Option<&str> {
let op = self.hugr.get_optype(self.parent());
Expand Down Expand Up @@ -269,12 +270,14 @@ impl<T: HugrView> Circuit<T> {
.sum()
}

/// Return the graphviz representation of the underlying graph and hierarchy side by side.
/// Return the graphviz representation of the underlying graph and hierarchy
/// side by side.
///
/// For a simpler representation, use the [`Circuit::mermaid_string`] format instead.
/// For a simpler representation, use the [`Circuit::mermaid_string`] format
/// instead.
pub fn dot_string(&self) -> String {
// TODO: This will print the whole HUGR without identifying the circuit container.
// Should we add some extra formatting for that?
// TODO: This will print the whole HUGR without identifying the circuit
// container. Should we add some extra formatting for that?
self.hugr.dot_string()
}

Expand Down Expand Up @@ -333,12 +336,14 @@ impl<T: HugrView<Node = Node>> Circuit<T> {
})
}

/// Extracts the circuit into a new owned HUGR containing the circuit at the root.
/// Replaces the circuit container operation with an [`OpType::DFG`].
/// Extracts the circuit into a new owned HUGR containing the circuit at the
/// root. Replaces the circuit container operation with an
/// [`OpType::DFG`].
///
/// Regions that are not descendants of the parent node are not included in the new HUGR.
/// This may invalidate calls to functions defined elsewhere. Make sure to inline any
/// external functions before calling this method.
/// Regions that are not descendants of the parent node are not included in
/// the new HUGR. This may invalidate calls to functions defined
/// elsewhere. Make sure to inline any external functions before calling
/// this method.
pub fn extract_dfg(&self) -> Result<Circuit<Hugr>, CircuitMutError> {
let circ = self.to_owned();
// TODO: Can we just ignore this now?
Expand Down
1 change: 1 addition & 0 deletions tket/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,4 @@ pub use circuit::{Circuit, CircuitError, CircuitMutError};
pub use hugr;
pub use hugr::Hugr;
pub use ops::{op_matches, symbolic_constant_op, Pauli, TketOp};
pub use subcircuit::Subcircuit;
13 changes: 5 additions & 8 deletions tket/src/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@

// Public API exports
pub use flow::{DefaultResourceFlow, ResourceFlow, UnsupportedOp};
pub use scope::{ResourceScope, ResourceScopeConfig};
pub use scope::ResourceScope;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there good reason to drop this public export?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, feel free to leave it :)

pub use types::{CircuitUnit, Position, ResourceAllocator, ResourceId};

// Internal modules
Expand All @@ -58,8 +58,6 @@ pub(crate) mod tests {
use hugr::{
builder::{DFGBuilder, Dataflow, DataflowHugr},
extension::prelude::qb_t,
hugr::views::SiblingSubgraph,
ops::handle::DataflowParentID,
types::Signature,
CircuitUnit, Hugr,
};
Expand All @@ -71,7 +69,7 @@ pub(crate) mod tests {
extension::rotation::{rotation_type, ConstRotation},
resource::scope::tests::ResourceScopeReport,
utils::build_simple_circuit,
TketOp,
Circuit, TketOp,
};

use super::ResourceScope;
Expand All @@ -89,7 +87,7 @@ pub(crate) mod tests {
}

// Gate being commuted has a non-linear input
fn circ(n_qubits: usize, add_rz: bool, add_const_rz: bool) -> Hugr {
pub fn cx_rz_circuit(n_qubits: usize, add_rz: bool, add_const_rz: bool) -> Hugr {
let build = || {
let out_qb_row = vec![qb_t(); n_qubits];
let mut inp_qb_row = out_qb_row.clone();
Expand Down Expand Up @@ -154,9 +152,8 @@ pub(crate) mod tests {
#[case] add_rz: bool,
#[case] add_const_rz: bool,
) {
let circ = circ(n_qubits, add_rz, add_const_rz);
let subgraph =
SiblingSubgraph::try_new_dataflow_subgraph::<_, DataflowParentID>(&circ).unwrap();
let circ = cx_rz_circuit(n_qubits, add_rz, add_const_rz);
let subgraph = Circuit::from(&circ).subgraph().unwrap();
let scope = ResourceScope::new(&circ, subgraph);
let info = ResourceScopeReport::from(&scope);

Expand Down
91 changes: 60 additions & 31 deletions tket/src/resource/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,6 @@ impl<H: HugrView> ResourceScope<H> {
Some(*port_map.get(port))
}

/// Get the [`ResourceId`] for a given port.
///
/// Return None if the port is not a resource port.
pub fn get_resource_id(&self, node: H::Node, port: impl Into<Port>) -> Option<ResourceId> {
let unit = self.get_circuit_unit(node, port)?;
unit.as_resource()
}

/// Get all [`CircuitUnit`]s for either the incoming or outgoing ports of a
/// node.
pub fn get_circuit_units_slice(
Expand All @@ -179,15 +171,47 @@ impl<H: HugrView> ResourceScope<H> {
Some(port_map.get_slice(direction))
}

/// Get the port of node on the given resource path.
/// Get the ports of node with the given opvalue in the given direction.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

with the given [CircuitUnit], I guess, but I'm more puzzled - why might there be more than one?

get_resource_port handles the linear case, and limits to at-most-one.

So I presume this means you might have multiple classical ports with the same unit....ah, if you are asking for inputs, and multiple inputs are wired from the same classical port? (But it'll never be >1 for Direction::Outgoing?)

Is that right? If so I can just add a comment

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that is exactly the point. For copyable types, you may have more than one incoming port connected to the same value (identified by (Node, OutgoingPort))

///
/// The returned port will have the direction `dir`.
pub fn get_port(&self, node: H::Node, resource_id: ResourceId, dir: Direction) -> Option<Port> {
let units = self.get_circuit_units_slice(node, dir)?;
let offset = units
.iter()
.position(|unit| unit.as_resource() == Some(resource_id))?;
Some(Port::new(dir, offset))
pub fn get_ports(
&self,
node: H::Node,
unit: impl Into<CircuitUnit<H::Node>>,
dir: Direction,
) -> impl Iterator<Item = Port> + '_ {
let exp_unit = unit.into();
let units = self.get_circuit_units_slice(node, dir);
let offsets = units
.into_iter()
.flatten()
.positions(move |unit| unit == &exp_unit);
offsets.map(move |offset| Port::new(dir, offset))
}

/// Get the port of node with the given resource in the given direction.
pub fn get_resource_port(
&self,
node: H::Node,
resource_id: ResourceId,
dir: Direction,
) -> Option<Port> {
self.get_ports(node, resource_id, dir)
.at_most_one()
.ok()
.expect("linear resource")
}

/// Get the resource ID at the given port of the given node.
Copy link
Contributor Author

@acl-cqc acl-cqc Nov 28, 2025

Choose a reason for hiding this comment

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

Note to self - so this has just moved

pub fn get_resource_id(&self, node: H::Node, port: impl Into<Port>) -> Option<ResourceId> {
let unit = self.get_circuit_unit(node, port)?;
unit.as_resource()
}

/// Get the copyable wire at the given port of the given node.
pub fn get_copyable_wire(&self, node: H::Node, port: impl Into<Port>) -> Option<Wire<H::Node>> {
let unit = self.get_circuit_unit(node, port)?;
unit.as_copyable_wire()
}

/// Get the position of the given node.
Expand Down Expand Up @@ -215,24 +239,21 @@ impl<H: HugrView> ResourceScope<H> {
.filter_map(|unit| unit.as_resource())
}

/// All resource IDs on the ports of `node`, in both directions.
pub fn get_all_resources(&self, node: H::Node) -> Vec<ResourceId> {
/// All resource IDs on the ports of `node`, in both directions, in the
/// order that they appear along the ports of `node`.
pub fn get_all_resources(&self, node: H::Node) -> impl Iterator<Item = ResourceId> + '_ {
let in_resources = self.get_resources(node, Direction::Incoming);
let out_resources = self.get_resources(node, Direction::Outgoing);
let mut all_resources = in_resources.chain(out_resources).collect_vec();
all_resources.sort_unstable();
all_resources.dedup();
all_resources.shrink_to_fit();
all_resources
in_resources.chain(out_resources).unique()
}

/// Whether the given node is the first node on the path of the given
/// resource.
pub fn is_resource_start(&self, node: H::Node, resource_id: ResourceId) -> bool {
self.get_port(node, resource_id, Direction::Outgoing)
self.get_resource_port(node, resource_id, Direction::Outgoing)
.is_some()
&& self
.get_port(node, resource_id, Direction::Incoming)
.get_resource_port(node, resource_id, Direction::Incoming)
.is_none()
}

Expand All @@ -246,7 +267,7 @@ impl<H: HugrView> ResourceScope<H> {
}

/// All copyable wires on the ports of `node` in the given direction.
pub fn get_copyable_wires(
pub fn all_copyable_wires(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I note this is called only once, with Direction::Outgoing, for which case it is exactly the same as get_copyable_ports.

Whereas all_copyable_wires(...., Direction::Incoming), which is never used, there is nothing to tell you which port of node the specified Wire is connected to...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I suggest either dropping this one (have just get_copyable_ports), or have two methods, one for inputs returning (IncomingPort, Wire)s and one for outputs returning just Wires.

(There is only one case where you pass a non-constant direction to get_copyable_ports and I am minded to rewrite that...)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes good spot, this is redundant. I agree that get_copyable_ports is sufficient.

&self,
node: H::Node,
dir: Direction,
Expand All @@ -258,6 +279,18 @@ impl<H: HugrView> ResourceScope<H> {
})
}

/// All ports of `node` in the given direction that are copyable.
pub fn get_copyable_ports(
&self,
node: H::Node,
dir: Direction,
) -> impl Iterator<Item = Port> + '_ {
let units = self.get_circuit_units_slice(node, dir);
let ports = self.hugr().node_ports(node, dir);
let units_ports = units.into_iter().flatten().zip(ports);
units_ports.filter_map(|(unit, port)| unit.is_copyable().then_some(port))
}

/// Iterate over the nodes on the resource path starting from the given
/// node in the given direction.
pub fn resource_path_iter(
Expand All @@ -267,7 +300,7 @@ impl<H: HugrView> ResourceScope<H> {
direction: Direction,
) -> impl Iterator<Item = H::Node> + '_ {
iter::successors(Some(start_node), move |&curr_node| {
let port = self.get_port(curr_node, resource_id, direction)?;
let port = self.get_resource_port(curr_node, resource_id, direction)?;
let (next_node, _) = self
.hugr()
.single_linked_port(curr_node, port)
Expand Down Expand Up @@ -734,11 +767,7 @@ pub(crate) mod tests {
.map(|(n, _)| n);

for h in first_hadamards {
let res = scope
.get_all_resources(h)
.into_iter()
.exactly_one()
.unwrap();
let res = scope.get_all_resources(h).exactly_one().ok().unwrap();
let nodes_on_path = scope.resource_path_iter(res, h, Direction::Outgoing);
let pos_on_path = nodes_on_path.map(|n| scope.get_position(n).unwrap());

Expand Down
23 changes: 16 additions & 7 deletions tket/src/resource/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! copyable values throughout a HUGR circuit, including resource identifiers,
//! positions, and the mapping structures that associate them with operations.

use derive_more::From;
use hugr::{
core::HugrNode, types::Signature, Direction, IncomingPort, OutgoingPort, Port, PortIndex, Wire,
};
Expand All @@ -21,7 +22,8 @@ pub struct ResourceId(usize);
impl ResourceId {
/// Create a new ResourceId.
///
/// This method should only be called by ResourceAllocator and tests.
/// ResourceIds should typically be obtained from [`ResourceAllocator`].
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll put this in some #[cfg(test)]

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you!

/// Only use this in testing.
pub(crate) fn new(id: usize) -> Self {
Self(id)
}
Expand All @@ -38,7 +40,7 @@ impl ResourceId {
/// Initially assigned as contiguous integers, they may become non-integer
/// when operations are inserted or removed.
#[derive(Clone, Copy, Default, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct Position(Rational64);
pub struct Position(pub(crate) Rational64);

impl std::fmt::Debug for Position {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Expand All @@ -51,8 +53,8 @@ impl Position {
///
/// This method should only be called by allocators and tests.
#[allow(unused)]
pub(super) fn new_integer(i: i64) -> Self {
Self(Rational64::from_integer(i))
pub(super) fn new_integer(numer: i64) -> Self {
Self(Rational64::from_integer(numer))
}

/// Get position as f64, rounded to the given precision.
Expand All @@ -69,12 +71,19 @@ impl Position {

/// A value associated with a dataflow port, identified either by a resource ID
/// (for linear values) or by its wire (for copyable values).
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
///
/// This can currently be converted to and from [`hugr::CircuitUnit`], but
/// linear wires are assigned to resources with typed resource IDs instead of
/// integers.
///
/// Equivalence with [`hugr::CircuitUnit`] is not guaranteed in the future: we
/// may expand expressivity, e.g. identifying copyable units by their ASTs.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, From)]
pub enum CircuitUnit<N: HugrNode> {
/// A linear resource.
Resource(ResourceId),
Resource(#[from] ResourceId),
/// A copyable value.
Copyable(Wire<N>),
Copyable(#[from] Wire<N>),
}

impl<N: HugrNode> CircuitUnit<N> {
Expand Down
22 changes: 12 additions & 10 deletions tket/src/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub mod trace;
pub use ecc_rewriter::ECCRewriter;

use derive_more::{From, Into};
use hugr::core::HugrNode;
use hugr::hugr::hugrmut::HugrMut;
use hugr::hugr::patch::simple_replace;
use hugr::hugr::views::sibling_subgraph::InvalidReplacement;
Expand All @@ -20,17 +21,18 @@ use hugr::{
use hugr::{Hugr, HugrView, Node};

use crate::circuit::Circuit;
pub use crate::Subcircuit;

/// A rewrite rule for circuits.
#[derive(Debug, Clone, From, Into)]
pub struct CircuitRewrite<N = Node>(SimpleReplacement<N>);

impl CircuitRewrite {
impl<N: HugrNode> CircuitRewrite<N> {
/// Create a new rewrite rule.
pub fn try_new(
subgraph: &SiblingSubgraph,
hugr: &impl HugrView<Node = Node>,
replacement: Circuit<impl HugrView<Node = Node>>,
subgraph: &SiblingSubgraph<N>,
hugr: &impl HugrView<Node = N>,
replacement: Circuit<impl HugrView<Node = hugr::Node>>,
) -> Result<Self, InvalidReplacement> {
let replacement = replacement
.extract_dfg()
Expand All @@ -50,7 +52,7 @@ impl CircuitRewrite {
}

/// The subgraph that is replaced.
pub fn subgraph(&self) -> &SiblingSubgraph {
pub fn subgraph(&self) -> &SiblingSubgraph<N> {
self.0.subgraph()
}

Expand All @@ -65,16 +67,16 @@ impl CircuitRewrite {
/// Two `CircuitRewrite`s can be composed if their invalidation sets are
/// disjoint.
#[inline]
pub fn invalidation_set(&self) -> impl Iterator<Item = Node> + '_ {
pub fn invalidation_set(&self) -> impl Iterator<Item = N> + '_ {
self.0.invalidation_set()
}

/// Apply the rewrite rule to a circuit.
#[inline]
pub fn apply(
self,
circ: &mut Circuit<impl HugrMut<Node = Node>>,
) -> Result<simple_replace::Outcome<Node>, SimpleReplacementError> {
circ: &mut Circuit<impl HugrMut<Node = N>>,
) -> Result<simple_replace::Outcome<N>, SimpleReplacementError> {
circ.add_rewrite_trace(&self);
self.0.apply(circ.hugr_mut())
}
Expand All @@ -83,8 +85,8 @@ impl CircuitRewrite {
#[inline]
pub fn apply_notrace(
self,
circ: &mut Circuit<impl HugrMut<Node = Node>>,
) -> Result<simple_replace::Outcome<Node>, SimpleReplacementError> {
circ: &mut Circuit<impl HugrMut<Node = N>>,
) -> Result<simple_replace::Outcome<N>, SimpleReplacementError> {
self.0.apply(circ.hugr_mut())
}
}
Expand Down
Loading
Loading