Green light?

Author: aborgna-q

tket/src/serialize/pytket/circuit.rs

@@ -7,7 +7,7 @@ use std::sync::Arc;
 use hugr::core::HugrNode;
 use hugr::hugr::hugrmut::HugrMut;
 use hugr::ops::handle::NodeHandle;
-use hugr::ops::{OpTag, OpTrait};
+use hugr::ops::{OpParent, OpTag, OpTrait};
 use hugr::{Hugr, HugrView, Node};
 use hugr_core::hugr::internal::HugrMutInternals;
 use itertools::Itertools;
@@ -34,6 +34,7 @@ use super::opaque::OpaqueSubgraphs;
 /// circuit that can be used independently, and stored permanently, use
 /// [`EncodedCircuit::new_standalone`] or call
 /// [`EncodedCircuit::ensure_standalone`].
+#[derive(Debug, Clone)]
 pub struct EncodedCircuit<Node: HugrNode> {
     /// Circuits encoded from independent dataflow regions in the HUGR.
     ///
@@ -93,7 +94,6 @@ impl EncodedCircuit<Node> {
         };
 
         enc.encode_circuits(circuit, options)?;
-        enc.ensure_standalone(circuit.hugr())?;
 
         Ok(enc)
     }
@@ -124,7 +124,7 @@ impl EncodedCircuit<Node> {
     /// Returns an error if a circuit being decoded is invalid. See
     /// [`PytketDecodeErrorInner`][super::error::PytketDecodeErrorInner] for
     /// more details.
-    pub fn reassemble_inline<H: AsRef<Hugr> + AsMut<Hugr> + HugrView<Node = Node>>(
+    pub fn reassemble_inline(
         &self,
         hugr: &mut Hugr,
         config: Option<Arc<PytketDecoderConfig>>,
@@ -136,7 +136,7 @@ impl EncodedCircuit<Node> {
 
         for (&original_region, encoded) in &self.circuits {
             // Decode the circuit into a temporary function node.
-            let Some(signature) = hugr.get_optype(original_region).dataflow_signature() else {
+            let Some(signature) = hugr.get_optype(original_region).inner_function_type() else {
                 return Err(PytketDecodeErrorInner::IncompatibleTargetRegion {
                     region: original_region,
                     new_optype: hugr.get_optype(original_region).clone(),
@@ -171,6 +171,7 @@ impl EncodedCircuit<Node> {
             while let Some(child) = hugr.first_child(decoded_node) {
                 hugr.set_parent(child, original_region);
             }
+            hugr.remove_node(decoded_node);
         }
         Ok(self.circuits.keys().copied().collect_vec())
     }
@@ -201,6 +202,7 @@ impl<Node: HugrNode> EncodedCircuit<Node> {
         };
 
         enc.encode_circuits(circuit, options)?;
+        enc.ensure_standalone(circuit.hugr())?;
 
         Ok(enc)
     }

tket/src/serialize/pytket/decoder.rs

@@ -63,7 +63,7 @@ pub struct PytketDecoderContext<'h> {
     options: DecodeOptions,
     /// A registry of opaque subgraphs from `original_hugr`, that are referenced by opaque barriers in the pytket circuit
     /// via their [`SubgraphId`].
-    opaque_subgraphs: Option<&'h OpaqueSubgraphs<Node>>,
+    pub(super) opaque_subgraphs: Option<&'h OpaqueSubgraphs<Node>>,
 }
 
 impl<'h> PytketDecoderContext<'h> {
@@ -331,8 +331,12 @@ impl<'h> PytketDecoderContext<'h> {
                 };
                 e.hugr_op("Output")
             })?;
+        let output_wire_count = output_wires.register_count();
         let output_wires = output_wires.wires();
 
+        // Qubits not in the output need to be freed.
+        self.add_implicit_qfree_operations(&qubits[output_wire_count.qubits..]);
+
         // Store the name for the input parameter wires
         let input_params = self.wire_tracker.finish();
         if !input_params.is_empty() {
@@ -349,6 +353,33 @@ impl<'h> PytketDecoderContext<'h> {
             .node())
     }
 
+    /// Add the implicit QFree operations for a list of qubits that are not in the hugr output.
+    ///
+    /// We only do this if there's a wire with type `qb_t` containing the qubit.
+    fn add_implicit_qfree_operations(&mut self, qubits: &[TrackedQubit]) {
+        let qb_type = qb_t();
+        let mut bit_args: &[TrackedBit] = &[];
+        let mut params: &[LoadedParameter] = &[];
+        for q in qubits.iter() {
+            let mut qubit_args: &[TrackedQubit] = std::slice::from_ref(q);
+            let Ok(FoundWire::Register(wire)) = self.wire_tracker.find_typed_wire(
+                self.config(),
+                &qb_type,
+                &mut qubit_args,
+                &mut bit_args,
+                &mut params,
+                None,
+            ) else {
+                continue;
+            };
+
+            self.builder
+                .add_dataflow_op(TketOp::QFree, [wire.wire()])
+                .unwrap()
+                .out_wire(0);
+        }
+    }
+
     /// Register the set of opaque subgraphs that are present in the HUGR being decoded.
     ///
     /// # Arguments

tket/src/serialize/pytket/decoder/subgraph.rs

@@ -69,10 +69,12 @@ impl<'h> PytketDecoderContext<'h> {
             .builder
             .hugr()
             .get_parent(subgraph.nodes()[0])
-            .ok_or_else(|| mk_subgraph_error(id, "Subgraph must contain be dataflow nodes."))?;
-        let [old_input, old_output] = self.builder.hugr().get_io(old_parent).ok_or_else(|| {
-            mk_subgraph_error(id, "Stored subgraph must be in a dataflow region.")
-        })?;
+            .ok_or_else(|| mk_subgraph_error(id, "Subgraph must contain dataflow nodes."))?;
+        let [old_input, old_output] = self
+            .builder
+            .hugr()
+            .get_io(old_parent)
+            .ok_or_else(|| mk_subgraph_error(id, "Subgraph must be in a dataflow region."))?;
         let new_parent = self.builder.container_node();
 
         // Re-parent the nodes in the subgraph.
@@ -99,7 +101,9 @@ impl<'h> PytketDecoderContext<'h> {
                     unreachable!("`unsupported_wire` not passed to `find_typed_wire`.");
                 }
                 Err(PytketDecodeError {
-                    inner: PytketDecodeErrorInner::NoMatchingWire { .. },
+                    inner:
+                        PytketDecodeErrorInner::NoMatchingWire { .. }
+                        | PytketDecodeErrorInner::NoMatchingParameter { .. },
                     ..
                 }) => {
                     // Not a qubit or bit wire.
@@ -130,7 +134,7 @@ impl<'h> PytketDecoderContext<'h> {
         // Re-wire wires from the subgraph to the old region's outputs.
         let mut output_qubits = qubits;
         let mut output_bits = bits;
-        for (ty, (src, src_port)) in signature.input().iter().zip_eq(subgraph.outgoing_ports()) {
+        for (ty, (src, src_port)) in signature.output().iter().zip_eq(subgraph.outgoing_ports()) {
             let wire = Wire::new(*src, *src_port);
             match self.config().type_to_pytket(ty) {
                 Some(counts) => {
@@ -307,6 +311,8 @@ impl<'h> PytketDecoderContext<'h> {
                             .entry(id)
                             .or_default()
                             .extend(targets);
+                        // TODO: We have to store this list somewhere so we
+                        // connect ports from subgraph that get added later.
                         continue;
                     };
                     *wire
@@ -350,14 +356,6 @@ impl<'h> PytketDecoderContext<'h> {
                 None => {
                     // This is an unsupported wire, so we just register the edge id to the wire.
                     self.wire_tracker.register_unsupported_wire(*edge_id, wire);
-                    // If we've registered a pending connection for this edge id, connect it now.
-                    if let Some(targets) = pending_encoded_edge_connections.remove(edge_id) {
-                        for (node, port) in targets {
-                            self.builder
-                                .hugr_mut()
-                                .connect(wire.node(), wire.source(), node, port);
-                        }
-                    }
                 }
             }
         }

tket/src/serialize/pytket/error.rs

@@ -348,7 +348,7 @@ pub enum PytketDecodeErrorInner {
         bit_args: Vec<String>,
     },
     /// We couldn't find a parameter for the required input type.
-    #[display("Could not find a parameter for the required input type {ty}")]
+    #[display("Could not find a parameter for the required input type '{ty}'")]
     NoMatchingParameter {
         /// The type that couldn't be found.
         ty: String,

tket/src/serialize/pytket/extension/core.rs

@@ -12,9 +12,9 @@ use crate::serialize::pytket::decoder::{
 use crate::serialize::pytket::extension::PytketDecoder;
 use crate::serialize::pytket::opaque::{OpaqueSubgraphPayload, OPGROUP_OPAQUE_HUGR};
 use crate::serialize::pytket::{DecodeInsertionTarget, DecodeOptions, PytketDecodeError};
-use crate::serialize::TKETDecode;
 use hugr::builder::Container;
 use hugr::extension::prelude::{bool_t, qb_t};
+use hugr::ops::handle::NodeHandle;
 use hugr::types::{Signature, Type};
 use itertools::Itertools;
 use tket_json_rs::circuit_json::Operation as PytketOperation;
@@ -57,7 +57,11 @@ impl PytketDecoder for CoreDecoder {
             }
             PytketOperation {
                 op_type: PytketOptype::CircBox,
-                op_box: Some(OpBox::CircBox { id: _id, circuit }),
+                op_box:
+                    Some(OpBox::CircBox {
+                        id: _id,
+                        circuit: serial_circuit,
+                    }),
                 ..
             } => {
                 // We have no way to distinguish between input and output bits
@@ -79,8 +83,19 @@ impl PytketDecoder for CoreDecoder {
                 let target = DecodeInsertionTarget::Region {
                     parent: decoder.builder.container_node(),
                 };
-                let internal =
-                    circuit.decode_inplace(decoder.builder.hugr_mut(), target, options)?;
+
+                // Decode the circuit box into a DFG node in the region.
+                let mut nested_decoder = PytketDecoderContext::new(
+                    serial_circuit,
+                    decoder.builder.hugr_mut(),
+                    target,
+                    options,
+                )?;
+                if let Some(opaque_subgraphs) = decoder.opaque_subgraphs {
+                    nested_decoder.register_opaque_subgraphs(opaque_subgraphs);
+                }
+                nested_decoder.run_decoder(&serial_circuit.commands)?;
+                let internal = nested_decoder.finish()?.node();
 
                 decoder
                     .wire_up_node(internal, qubits, qubits, bits, bits, params)

tket/src/serialize/pytket/opaque.rs

@@ -15,12 +15,12 @@ use hugr::HugrView;
 
 /// The ID of a subgraph in the Hugr.
 #[derive(Debug, derive_more::Display, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-#[display("{local_id}.{tracker_id}")]
+#[display("{tracker_id}.{local_id}")]
 pub struct SubgraphId {
-    /// A locally unique ID in the [`OpaqueSubgraphs`] instance.
-    local_id: usize,
     /// The unique ID of the [`OpaqueSubgraphs`] instance that generated this ID.
     tracker_id: usize,
+    /// A locally unique ID in the [`OpaqueSubgraphs`] instance.
+    local_id: usize,
 }
 
 /// A set of subgraphs a HUGR that have been marked as _unsupported_ during a
@@ -44,16 +44,16 @@ pub(super) struct OpaqueSubgraphs<N> {
 
 impl serde::Serialize for SubgraphId {
     fn serialize<S: serde::Serializer>(&self, s: S) -> Result<S::Ok, S::Error> {
-        (&self.local_id, &self.tracker_id).serialize(s)
+        (&self.tracker_id, &self.local_id).serialize(s)
     }
 }
 
 impl<'de> serde::Deserialize<'de> for SubgraphId {
     fn deserialize<D: serde::Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
-        let (local_id, tracker_id) = serde::Deserialize::deserialize(d)?;
+        let (tracker_id, local_id) = serde::Deserialize::deserialize(d)?;
         Ok(Self {
-            local_id,
             tracker_id,
+            local_id,
         })
     }
 }

tket/src/serialize/pytket/tests.rs

@@ -594,18 +594,19 @@ fn json_file_roundtrip(#[case] circ: impl AsRef<std::path::Path>) {
     compare_serial_circs(&ser, &reser);
 }
 
-/// Test the serialisation roundtrip from a tket circuit.
+/// Test the standalone serialisation roundtrip from a tket circuit.
 ///
-/// Note: this is not a pure roundtrip as the encoder may add internal qubits/bits to the circuit.
+/// This is not a pure roundtrip as the encoder may add internal qubits/bits to
+/// the circuit.
+///
+/// Standalone circuit do not currently support unsupported subgraphs with
+/// nested structure or non-local edges.
 #[rstest]
 #[case::meas_ancilla(circ_measure_ancilla(), 1)]
 #[case::preset_qubits(circ_preset_qubits(), 1)]
 #[case::preset_parameterized(circ_parameterized(), 1)]
 #[case::nested_dfgs(circ_nested_dfgs(), 1)]
-#[case::global_defs(circ_global_defs(), 1)]
-#[case::recursive(circ_recursive(), 1)]
-#[case::non_local(circ_non_local(), 1)]
-fn circuit_roundtrip(#[case] circ: Circuit, #[case] num_circuits: usize) {
+fn circuit_standalone_roundtrip(#[case] circ: Circuit, #[case] num_circuits: usize) {
     let circ_signature = circ.circuit_signature().into_owned();
 
     let encoded = EncodedCircuit::new_standalone(&circ, EncodeOptions::new_with_subcircuits())
@@ -619,6 +620,8 @@ fn circuit_roundtrip(#[case] circ: Circuit, #[case] num_circuits: usize) {
         .decode(DecodeOptions::new().with_signature(circ_signature.clone()))
         .unwrap_or_else(|e| panic!("{e}"));
 
+    deser.hugr().validate().unwrap_or_else(|e| panic!("{e}"));
+
     let deser_sig = deser.circuit_signature();
     assert_eq!(
         &circ_signature.input, &deser_sig.input,
@@ -636,6 +639,46 @@ fn circuit_roundtrip(#[case] circ: Circuit, #[case] num_circuits: usize) {
     compare_serial_circs(ser, &reser);
 }
 
+/// Test the serialisation roundtrip from a tket circuit into an EncodedCircuit and back.
+#[rstest]
+#[case::meas_ancilla(circ_measure_ancilla(), 1)]
+#[case::preset_qubits(circ_preset_qubits(), 1)]
+#[case::preset_parameterized(circ_parameterized(), 1)]
+#[case::nested_dfgs(circ_nested_dfgs(), 1)]
+#[case::global_defs(circ_global_defs(), 1)]
+#[case::recursive(circ_recursive(), 1)]
+// TODO: fix edge case: non-local edge from an unsupported node inside a nested CircBox
+// to/from the input of the head region being encoded...
+//#[case::non_local(circ_non_local(), 1)]
+fn encoded_circuit_roundtrip(#[case] circ: Circuit, #[case] num_circuits: usize) {
+    let circ_signature = circ.circuit_signature().into_owned();
+
+    let encoded = EncodedCircuit::new(&circ, EncodeOptions::new_with_subcircuits())
+        .unwrap_or_else(|e| panic!("{e}"));
+
+    assert!(encoded.contains_circuit(circ.parent()));
+    assert_eq!(encoded.len(), num_circuits);
+
+    let mut deser = circ.clone();
+    encoded
+        .reassemble_inline(deser.hugr_mut(), None)
+        .unwrap_or_else(|e| panic!("{e}"));
+
+    deser.hugr().validate().unwrap_or_else(|e| panic!("{e}"));
+
+    let deser_sig = deser.circuit_signature();
+    assert_eq!(
+        &circ_signature.input, &deser_sig.input,
+        "Input signature mismatch\n  Expected: {}\n  Actual:   {}",
+        &circ_signature, &deser_sig
+    );
+    assert_eq!(
+        &circ_signature.output, &deser_sig.output,
+        "Output signature mismatch\n  Expected: {}\n  Actual:   {}",
+        &circ_signature, &deser_sig
+    );
+}
+
 /// Test serialisation of circuits with a symbolic expression.
 ///
 /// Note: this is not a proper roundtrip as the symbols f0 and f1 are not