Test standalone circ roundtrip with unsupported graph, and test errors

Author: aborgna-q

tket/src/passes/pytket.rs

@@ -7,7 +7,7 @@ use derive_more::{Display, Error, From};
 use hugr::{HugrView, Node};
 use itertools::Itertools;
 
-use crate::serialize::pytket::OpConvertError;
+use crate::serialize::pytket::PytketEncodeOpError;
 use crate::Circuit;
 
 use super::find_tuple_unpack_rewrites;
@@ -37,7 +37,7 @@ pub enum PytketLoweringError {
     /// An error occurred during the conversion of an operation.
     #[display("operation conversion error: {_0}")]
     #[from]
-    OpConversionError(OpConvertError),
+    OpConversionError(PytketEncodeOpError),
     /// The circuit is not fully-contained in a region.
     /// Function calls are not supported.
     #[display("Non-local operations found. Function calls are not supported.")]

tket/src/serialize/pytket.rs

@@ -15,7 +15,9 @@ pub use config::{
     TypeTranslatorSet,
 };
 pub use encoder::PytketEncoderContext;
-pub use error::{OpConvertError, PytketDecodeError, PytketDecodeErrorInner, PytketEncodeError};
+pub use error::{
+    PytketDecodeError, PytketDecodeErrorInner, PytketEncodeError, PytketEncodeOpError,
+};
 pub use extension::PytketEmitter;
 pub use options::{DecodeInsertionTarget, DecodeOptions, EncodeOptions};
 

tket/src/serialize/pytket/encoder.rs

@@ -27,7 +27,7 @@ use unsupported_tracker::UnsupportedTracker;
 
 use super::opaque::OpaqueSubgraphs;
 use super::{
-    OpConvertError, PytketEncodeError, METADATA_OPGROUP, METADATA_PHASE, METADATA_Q_REGISTERS,
+    PytketEncodeError, PytketEncodeOpError, METADATA_OPGROUP, METADATA_PHASE, METADATA_Q_REGISTERS,
 };
 use crate::circuit::Circuit;
 use crate::serialize::pytket::circuit::EncodedCircuitInfo;
@@ -313,7 +313,7 @@ impl<H: HugrView> PytketEncoderContext<H> {
             return self.get_wire_values(wire, circ);
         }
 
-        Err(OpConvertError::WireHasNoValues { wire }.into())
+        Err(PytketEncodeOpError::WireHasNoValues { wire }.into())
     }
 
     /// Given a node in the HUGR, returns all the [`TrackedValue`]s associated
@@ -378,7 +378,7 @@ impl<H: HugrView> PytketEncoderContext<H> {
 
             match self.get_wire_values(wire, circ) {
                 Ok(values) => tracked_values.extend(values.iter().copied()),
-                Err(PytketEncodeError::OpConversionError(OpConvertError::WireHasNoValues {
+                Err(PytketEncodeError::OpEncoding(PytketEncodeOpError::WireHasNoValues {
                     wire,
                 })) => unknown_values.push(wire),
                 Err(e) => panic!(
@@ -1186,7 +1186,7 @@ impl<N: HugrNode> NodeInputValues<N> {
     pub fn try_into_tracked_values(self) -> Result<TrackedValues, PytketEncodeError<N>> {
         match self.unknown_values.is_empty() {
             true => Ok(self.tracked_values),
-            false => Err(OpConvertError::WireHasNoValues {
+            false => Err(PytketEncodeOpError::WireHasNoValues {
                 wire: self.unknown_values[0],
             }
             .into()),

tket/src/serialize/pytket/encoder/value_tracker.rs

@@ -22,7 +22,7 @@ use tket_json_rs::register::ElementId as RegisterUnit;
 use crate::circuit::Circuit;
 use crate::serialize::pytket::extension::RegisterCount;
 use crate::serialize::pytket::{
-    OpConvertError, PytketEncodeError, RegisterHash, METADATA_B_REGISTERS,
+    PytketEncodeError, PytketEncodeOpError, RegisterHash, METADATA_B_REGISTERS,
     METADATA_INPUT_PARAMETERS,
 };
 
@@ -315,7 +315,7 @@ impl<N: HugrNode> ValueTracker<N> {
         wire: Wire<N>,
         values: impl IntoIterator<Item = Val>,
         circ: &Circuit<impl HugrView<Node = N>>,
-    ) -> Result<(), OpConvertError<N>> {
+    ) -> Result<(), PytketEncodeOpError<N>> {
         let values = values.into_iter().map(|v| v.into()).collect_vec();
 
         // Remove any qubit/bit used here from the unused set.
@@ -337,7 +337,7 @@ impl<N: HugrNode> ValueTracker<N> {
             unexplored_neighbours,
         };
         if self.wires.insert(wire, tracked).is_some() {
-            return Err(OpConvertError::WireAlreadyHasValues { wire });
+            return Err(PytketEncodeOpError::WireAlreadyHasValues { wire });
         }
 
         if unexplored_neighbours == 0 {
@@ -429,7 +429,7 @@ impl<N: HugrNode> ValueTracker<N> {
         self,
         circ: &Circuit<impl HugrView<Node = N>>,
         region: N,
-    ) -> Result<ValueTrackerResult, OpConvertError<N>> {
+    ) -> Result<ValueTrackerResult, PytketEncodeOpError<N>> {
         let output_node = circ.hugr().get_io(region).unwrap()[1];
 
         // Ordered list of qubits and bits at the output of the circuit.
@@ -440,7 +440,7 @@ impl<N: HugrNode> ValueTracker<N> {
             let wire = Wire::new(node, port);
             let values = self
                 .peek_wire_values(wire)
-                .ok_or_else(|| OpConvertError::WireHasNoValues { wire })?;
+                .ok_or_else(|| PytketEncodeOpError::WireHasNoValues { wire })?;
             for value in values {
                 match value {
                     TrackedValue::Qubit(qb) => qubit_outputs.push(self.qubit_register(*qb).clone()),

tket/src/serialize/pytket/error.rs

@@ -15,7 +15,7 @@ use crate::serialize::pytket::opaque::SubgraphId;
 #[derive(Display, derive_more::Debug, Error)]
 #[non_exhaustive]
 #[debug(bounds(N: HugrNode))]
-pub enum OpConvertError<N = hugr::Node> {
+pub enum PytketEncodeOpError<N: HugrNode = hugr::Node> {
     /// Tried to decode a tket1 operation with not enough parameters.
     #[display(
         "Operation {} is missing encoded parameters. Expected at least {expected} but only \"{}\" were specified.",
@@ -48,7 +48,7 @@ pub enum OpConvertError<N = hugr::Node> {
     /// Tried to query the values associated with an unexplored wire.
     ///
     /// This reflects a bug in the operation encoding logic of an operation.
-    #[display("Could not find values associated with wire {wire}.")]
+    #[display("Could not find values associated with {wire}.")]
     WireHasNoValues {
         /// The wire that has no values.
         wire: Wire<N>,
@@ -61,13 +61,21 @@ pub enum OpConvertError<N = hugr::Node> {
         /// The wire that already has values.
         wire: Wire<N>,
     },
+    /// Cannot encode subgraphs with nested structure or non-local edges in an standalone circuit.
+    #[display("Cannot encode subgraphs with nested structure or non-local edges in an standalone circuit. Unsupported nodes: {}",
+        nodes.iter().join(", "),
+    )]
+    UnsupportedStandaloneSubgraph {
+        /// The nodes that are part of the unsupported subgraph.
+        nodes: Vec<N>,
+    },
 }
 
 /// Error type for conversion between tket ops and pytket operations.
 #[derive(derive_more::Debug, Display, Error, From)]
 #[non_exhaustive]
 #[debug(bounds(N: HugrNode))]
-pub enum PytketEncodeError<N = hugr::Node> {
+pub enum PytketEncodeError<N: HugrNode = hugr::Node> {
     /// Tried to encode a non-dataflow region.
     #[display("Cannot encode non-dataflow region at {region} with type {optype}.")]
     NonDataflowRegion {
@@ -78,7 +86,7 @@ pub enum PytketEncodeError<N = hugr::Node> {
     },
     /// Operation conversion error.
     #[from]
-    OpConversionError(OpConvertError<N>),
+    OpEncoding(PytketEncodeOpError<N>),
     /// Custom user-defined error raised while encoding an operation.
     #[display("Error while encoding operation: {msg}")]
     CustomError {
@@ -98,7 +106,7 @@ pub enum PytketEncodeError<N = hugr::Node> {
     UnsupportedSubgraphHasNoRegisters {},
 }
 
-impl<N> PytketEncodeError<N> {
+impl<N: HugrNode> PytketEncodeError<N> {
     /// Create a new error with a custom message.
     pub fn custom(msg: impl ToString) -> Self {
         Self::CustomError {

tket/src/serialize/pytket/opaque.rs

@@ -138,7 +138,7 @@ impl<N: HugrNode> OpaqueSubgraphs<N> {
             return Err(PytketEncodeError::custom(format!("Barrier operation with opgroup {OPGROUP_OPAQUE_HUGR} points to an unknown subgraph: {subgraph_id}")));
         }
 
-        let payload = OpaqueSubgraphPayload::new_inline(&self[subgraph_id], hugr);
+        let payload = OpaqueSubgraphPayload::new_inline(&self[subgraph_id], hugr)?;
         command.op.data = Some(serde_json::to_string(&payload).unwrap());
 
         Ok(())
@@ -171,7 +171,9 @@ impl<N> Default for OpaqueSubgraphs<N> {
 /// # Errors
 ///
 /// Returns an error if the payload is invalid.
-fn parse_external_payload<N>(payload: &str) -> Result<Option<SubgraphId>, PytketEncodeError<N>> {
+fn parse_external_payload<N: HugrNode>(
+    payload: &str,
+) -> Result<Option<SubgraphId>, PytketEncodeError<N>> {
     // Check if the payload is inline, without fully copying it to memory.
     #[derive(serde::Deserialize)]
     struct PartialPayload {

tket/src/serialize/pytket/opaque/payload.rs

@@ -9,7 +9,9 @@ use hugr::package::Package;
 use hugr::types::Type;
 use hugr::{HugrView, Wire};
 
-use crate::serialize::pytket::{PytketDecodeError, PytketDecodeErrorInner};
+use crate::serialize::pytket::{
+    PytketDecodeError, PytketDecodeErrorInner, PytketEncodeError, PytketEncodeOpError,
+};
 
 use super::SubgraphId;
 
@@ -108,12 +110,28 @@ impl OpaqueSubgraphPayload {
     /// Create a new payload for an opaque subgraph in the Hugr.
     ///
     /// Encodes the subgraph into a hugr envelope.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if a node in the subgraph has children or non-local const edges.
     pub fn new_inline<N: HugrNode>(
         subgraph: &SiblingSubgraph<N>,
         hugr: &impl HugrView<Node = N>,
-    ) -> Self {
+    ) -> Result<Self, PytketEncodeError<N>> {
         let signature = subgraph.signature(hugr);
 
+        if !subgraph.function_calls().is_empty()
+            || subgraph
+                .nodes()
+                .iter()
+                .any(|n| hugr.children(*n).next().is_some())
+        {
+            return Err(PytketEncodeOpError::UnsupportedStandaloneSubgraph {
+                nodes: subgraph.nodes().to_vec(),
+            }
+            .into());
+        }
+
         let mut inputs = Vec::with_capacity(subgraph.incoming_ports().iter().map(Vec::len).sum());
         for subgraph_inputs in subgraph.incoming_ports() {
             let Some((inp_node, inp_port0)) = subgraph_inputs.first() else {
@@ -135,11 +153,11 @@ impl OpaqueSubgraphPayload {
             .store_str(EnvelopeConfig::text())
             .unwrap();
 
-        Self::Inline {
+        Ok(Self::Inline {
             hugr_envelope,
             inputs: signature.input().iter().cloned().zip(inputs).collect(),
             outputs: signature.output().iter().cloned().zip(outputs).collect(),
-        }
+        })
     }
 
     /// Load a payload encoded in a json string.

tket/src/serialize/pytket/tests.rs

@@ -7,11 +7,11 @@ use hugr::builder::{
     Container, Dataflow, DataflowHugr, DataflowSubContainer, FunctionBuilder, HugrBuilder,
     ModuleBuilder, SubContainer,
 };
-use hugr::extension::prelude::{bool_t, qb_t};
+use hugr::extension::prelude::{bool_t, option_type, qb_t, UnwrapBuilder};
 
 use hugr::hugr::hugrmut::HugrMut;
 use hugr::ops::handle::FuncID;
-use hugr::ops::{OpParent, Value};
+use hugr::ops::{OpParent, OpType, Value};
 use hugr::std_extensions::arithmetic::float_ops::FloatOps;
 use hugr::types::Signature;
 use hugr::HugrView;
@@ -27,8 +27,9 @@ use crate::extension::bool::BoolOp;
 use crate::extension::rotation::{rotation_type, ConstRotation, RotationOp};
 use crate::extension::sympy::SympyOpDef;
 use crate::extension::TKET1_EXTENSION_ID;
+use crate::serialize::pytket::extension::OpaqueTk1Op;
 use crate::serialize::pytket::{
-    DecodeInsertionTarget, DecodeOptions, EncodeOptions, EncodedCircuit,
+    DecodeInsertionTarget, DecodeOptions, EncodeOptions, EncodedCircuit, PytketEncodeOpError,
 };
 use crate::TketOp;
 
@@ -277,6 +278,51 @@ fn circ_parameterized() -> Circuit {
     hugr.into()
 }
 
+/// A circuit with a nested unsupported operation.
+///
+/// Tries to allocate a qubit, and panics if it fails.
+/// This creates an unsupported conditional inside the region.
+#[fixture]
+fn circ_flat_opaque() -> Circuit {
+    let input_t = vec![qb_t(), qb_t()];
+    let output_t = vec![qb_t(), qb_t()];
+    let mut h = FunctionBuilder::new("flat_opaque", Signature::new(input_t, output_t)).unwrap();
+
+    let [q1, q2] = h.input_wires_arr();
+
+    // An unsupported tk1-only operation.
+    let mut tk1op = tket_json_rs::circuit_json::Operation::default();
+    tk1op.op_type = tket_json_rs::optype::OpType::CH;
+    tk1op.n_qb = Some(2);
+    let op: OpType = OpaqueTk1Op::new_from_op(&tk1op, 2, 0)
+        .as_extension_op()
+        .into();
+    let [q1, q2] = h.add_dataflow_op(op, [q1, q2]).unwrap().outputs_arr();
+
+    let hugr = h.finish_hugr_with_outputs([q1, q2]).unwrap();
+    hugr.into()
+}
+
+/// A circuit with a nested unsupported operation.
+///
+/// Tries to allocate a qubit, and panics if it fails.
+/// This creates an unsupported conditional inside the region.
+#[fixture]
+fn circ_nested_opaque() -> Circuit {
+    let input_t = vec![];
+    let output_t = vec![qb_t()];
+    let mut h = FunctionBuilder::new("nested_opaque", Signature::new(input_t, output_t)).unwrap();
+
+    let [maybe_q] = h
+        .add_dataflow_op(TketOp::TryQAlloc, [])
+        .unwrap()
+        .outputs_arr();
+    let [q] = h.build_unwrap_sum(1, option_type(qb_t()), maybe_q).unwrap();
+
+    let hugr = h.finish_hugr_with_outputs([q]).unwrap();
+    hugr.into()
+}
+
 /// A circuit with a recursive function call.
 #[fixture]
 fn circ_recursive() -> Circuit {
@@ -606,6 +652,7 @@ fn json_file_roundtrip(#[case] circ: impl AsRef<std::path::Path>) {
 #[case::preset_qubits(circ_preset_qubits(), 1)]
 #[case::preset_parameterized(circ_parameterized(), 1)]
 #[case::nested_dfgs(circ_nested_dfgs(), 1)]
+#[case::flat_opaque(circ_flat_opaque(), 1)]
 fn circuit_standalone_roundtrip(#[case] circ: Circuit, #[case] num_circuits: usize) {
     let circ_signature = circ.circuit_signature().into_owned();
 
@@ -639,12 +686,35 @@ fn circuit_standalone_roundtrip(#[case] circ: Circuit, #[case] num_circuits: usi
     compare_serial_circs(ser, &reser);
 }
 
+/// Test that more complex unsupported subgraphs (nested structure, non-local edges) are rejected when encoding a standalone circuit.
+#[rstest]
+//#[case::nested_opaque(circ_nested_opaque())] TODO: Raises a different error
+#[case::global_defs(circ_global_defs())]
+#[case::recursive(circ_recursive())]
+fn test_complex_unsupported_subgraphs(#[case] circ: Circuit) {
+    use cool_asserts::assert_matches;
+
+    use crate::serialize::pytket::PytketEncodeError;
+
+    println!("{}", circ.mermaid_string());
+
+    let try_encoded = EncodedCircuit::new_standalone(&circ, EncodeOptions::new());
+    assert_matches!(
+        try_encoded,
+        Err(PytketEncodeError::OpEncoding(
+            PytketEncodeOpError::UnsupportedStandaloneSubgraph { .. }
+        ))
+    );
+}
+
 /// 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::flat_opaque(circ_flat_opaque(), 1)]
+//#[case::nested_opaque(circ_nested_opaque(), 1)] TODO: Raises a different error
 #[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