Skip to content
45 changes: 31 additions & 14 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ clap = "4.5.53"
crossbeam-channel = "0.5.15"
csv = "1.4.0"
delegate = "0.13.5"
derive_more = "2.0.1"
derive_more = "2.1.0"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Drive-by bump so I don't have to write a workaround for derive_more#454

Copy link
Contributor

Choose a reason for hiding this comment

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

I reckon I've gone to some massive effort to avoid this in some of my other PR(/attempt)s....great to see that! :)

fxhash = "0.2.1"
indexmap = "2.12.1"
lazy_static = "1.5.0"
Expand Down
22 changes: 15 additions & 7 deletions tket/src/serialize/pytket/circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,28 @@ pub(super) struct EncodedCircuitInfo {
/// pytket circuit, as they cannot be attached to a pytket command.
#[derive(Debug, Clone)]
pub(super) struct AdditionalNodesAndWires {
/// A subgraph of the region that does not contain any operation encodable
/// as a pytket command, and has no qubit/bits in its boundary that could be
/// used to emit an opaque barrier command in the [`serial_circuit`].
pub extra_subgraph: Option<SubgraphId>,
/// Parameter expression inputs to the `extra_subgraph`.
/// These cannot be encoded either if there's no pytket command to attach them to.
pub extra_subgraph_params: Vec<String>,
/// Subgraphs of the region that could not be encoded as a pytket commands,
/// and have no qubit/bits in their boundary that could be used to emit an
/// opaque barrier command in the [`serial_circuit`].
pub additional_subgraphs: Vec<AdditionalSubgraph>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does there ever need to be more than one? If there are no qubit wires between the "multiple subgraphs", that can just be one big subgraph, no? And if there are qubit wires, then the subgraphs to which those wires are input, become barriers?

The subgraphs we extract are always convex. We could technically remove that invariant and merge different subgraphs together, but at the end of the day they are just sets of nodes so we don't win anything from doing that (And it complicates the logic of ID tracking).

Combining them together seems very much like what we do with barriers ATM

This is using the exact same behaviour we use for barriers;
when we require the register/param in an edge coming from an unsupported op we do a greedy BFS extracting all connected not-yet-extracted nodes we have seen.

This produces convex subgraphs (due to the toposort traversal of nodes), but may produce multiple subgraphs in some cases.

E.g, if we have

graph LR
    subgraph 0 ["(0) Module"]
        direction LR
        subgraph 1 ["(1) [**FuncDefn: #quot;test#quot;**]"]
            direction LR
            style 1 stroke:#832561,stroke-width:3px
            16["(16) Unsupported 1"]
            17["(17) Unsupported 2"]
            18["(18) Supported 1"]
            19["(19) Supported 2"]
            16-->18
            17-->19
            16-->17
        end
    end
Loading

and we extract nodes in order 16,18,17,19, we'll create two subgraphs where one containing {16,17} would have sufficed.

This doesn't really worsen anything. We'll just have two barriers / two external subgraphs instead of one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, didn't realize that the algorithm only took nodes already found and marked unsupported in the traversal but not yet put in a subgraph. So the alternative algorithm of (1) partitioning nodes into two sets, and (2) trying to build maximal convex subgraphs from the unsupported nodes, would give fewer barriers, including at most one initial subgraph....but ok, this is no big deal.

Might be worth mentioning in a comment that all elements in the Vec here could have been combined into one subgraph in case we ever come back to this (I don't think it's worth raising an issue!).

(That is necessarily true, right? Any unsupported op after a supported one will become a barrier, not part of the additional/initial block...right?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, if it comes after then it is connected by a bit/qubit so it will appear as a barrier inline.

/// List of wires that directly connected the input node to the output node in the encoded region,
/// and were not encoded in [`serial_circuit`].
///
/// We just store the input nodes's output port and output node's input port here.
pub straight_through_wires: Vec<StraightThroughWire>,
}

/// A subgraph of the encoded circuit that could not be associated to any qubit or bit register in the pytket circuit.
#[derive(Debug, Clone)]
pub(super) struct AdditionalSubgraph {
/// The subgraph of the region that could not be encoded as a pytket command,
/// and has no qubit/bits in its boundary that could be used to emit an opaque
/// barrier command in the [`serial_circuit`].
pub id: SubgraphId,
/// Parameter expression inputs to the `subgraph`.
pub params: Vec<String>,
}

/// A wire stored in the [`EncodedCircuitInfo`] that directly connected the
/// input node to the output node in the encoded region, and was not encoded in
/// the pytket circuit.
Expand Down
8 changes: 4 additions & 4 deletions tket/src/serialize/pytket/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,14 +477,14 @@ impl<'h> PytketDecoderContext<'h> {
// Add additional subgraphs and wires not encoded in commands.
let [input_node, output_node] = self.builder.io();
if let Some(extras) = extra_nodes_and_wires {
if let Some(subgraph_id) = extras.extra_subgraph {
let params = extras
.extra_subgraph_params
for extra_subgraph in &extras.additional_subgraphs {
let params = extra_subgraph
.params
.iter()
.map(|p| self.load_half_turns(p))
.collect_vec();

self.insert_external_subgraph(subgraph_id, &[], &[], &params)
self.insert_external_subgraph(extra_subgraph.id, &[], &[], &params)
.map_err(|e| e.hugr_op("External subgraph"))?;
}

Expand Down
66 changes: 29 additions & 37 deletions tket/src/serialize/pytket/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use hugr::ops::{OpTrait, OpType};
use hugr::types::EdgeKind;

use std::borrow::Cow;
use std::collections::{BTreeSet, HashMap};
use std::collections::HashMap;
use std::ops::RangeTo;
use std::sync::{Arc, RwLock};

Expand All @@ -30,7 +30,9 @@ use super::{
PytketEncodeError, PytketEncodeOpError, METADATA_OPGROUP, METADATA_PHASE, METADATA_Q_REGISTERS,
};
use crate::circuit::Circuit;
use crate::serialize::pytket::circuit::{AdditionalNodesAndWires, EncodedCircuitInfo};
use crate::serialize::pytket::circuit::{
AdditionalNodesAndWires, AdditionalSubgraph, EncodedCircuitInfo,
};
use crate::serialize::pytket::config::PytketEncoderConfig;
use crate::serialize::pytket::extension::RegisterCount;
use crate::serialize::pytket::opaque::{OpaqueSubgraph, OpaqueSubgraphPayload};
Expand All @@ -56,6 +58,11 @@ pub struct PytketEncoderContext<H: HugrView> {
unsupported: UnsupportedTracker<H::Node>,
/// A registry of already-encoded opaque subgraphs.
opaque_subgraphs: OpaqueSubgraphs<H::Node>,
/// Subgraphs in `opaque_subgraphs` that could not be emitted as opaque barriers
/// barrier, and must be stored in the [`EncodedCircuitInfo`] instead when
/// finishing the encoding.
/// Identified by their [`super::opaque::SubgraphId`] in `opaque_subgraphs`.
non_emitted_subgraphs: Vec<AdditionalSubgraph>,
/// Configuration for the encoding.
///
/// Contains custom operation/type/const emitters.
Expand Down Expand Up @@ -176,6 +183,7 @@ impl<H: HugrView> PytketEncoderContext<H> {
values: ValueTracker::new(circ, region, &config)?,
unsupported: UnsupportedTracker::new(circ),
opaque_subgraphs,
non_emitted_subgraphs: vec![],
config,
function_cache: Arc::new(RwLock::new(HashMap::new())),
})
Expand Down Expand Up @@ -224,30 +232,11 @@ impl<H: HugrView> PytketEncoderContext<H> {
region: H::Node,
) -> Result<(EncodedCircuitInfo, OpaqueSubgraphs<H::Node>), PytketEncodeError<H::Node>> {
// Add any remaining unsupported nodes
let mut extra_subgraph: Option<BTreeSet<H::Node>> = None;
let mut extra_subgraph_params = Vec::new();
while !self.unsupported.is_empty() {
let node = self.unsupported.iter().next().unwrap();
let opaque_subgraphs = self.unsupported.extract_component(node, circ.hugr())?;
match self.emit_unsupported(&opaque_subgraphs, circ) {
Ok(()) => (),
Err(PytketEncodeError::UnsupportedSubgraphHasNoRegisters { params }) => {
// We'll store the nodes in the `extra_subgraph` field of the `EncodedCircuitInfo`.
// So the decoder can reconstruct the original subgraph.
extra_subgraph
.get_or_insert_default()
.extend(opaque_subgraphs.nodes().iter().cloned());
extra_subgraph_params.extend(params);
}
Err(e) => return Err(e),
}
self.emit_unsupported(&opaque_subgraphs, circ)?;
}
let extra_subgraph = extra_subgraph
.map(|nodes| -> Result<_, PytketEncodeError<H::Node>> {
let subgraph = OpaqueSubgraph::try_from_nodes(nodes, circ.hugr())?;
Ok(self.opaque_subgraphs.register_opaque_subgraph(subgraph))
})
.transpose()?;

let tracker_result = self.values.finish(circ, region)?;

Expand All @@ -264,8 +253,7 @@ impl<H: HugrView> PytketEncoderContext<H> {
input_params: tracker_result.input_params,
output_params: tracker_result.params,
additional_nodes_and_wires: AdditionalNodesAndWires {
extra_subgraph,
extra_subgraph_params,
additional_subgraphs: self.non_emitted_subgraphs,
straight_through_wires: tracker_result.straight_through_wires,
},
};
Expand Down Expand Up @@ -653,24 +641,28 @@ impl<H: HugrView> PytketEncoderContext<H> {
}

// Check that we have qubits or bits to attach the barrier command to.
//
// This should only fail when looking at the "leftover" unsupported nodes at the end of the decoding process.
if op_values.qubits.is_empty() && op_values.bits.is_empty() {
return Err(PytketEncodeError::UnsupportedSubgraphHasNoRegisters {
// We cannot associate this subgraph to any qubit or bit register in the pytket circuit,
// so we'll store it in the [`AdditionalSubgraph`]s instead when finishing the encoding.
self.non_emitted_subgraphs.push(AdditionalSubgraph {
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be as simple as, just add the nodes to the unique additional-subgraph, perhaps with some assert that the resulting subgraph is convex - obviously if that assert breaks then my supposition is false, but ATM I don't see how it can.

(A qubit wire directly from one unsupported bit to another is OK I guess, as long as there are no supported ops on said wire.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't store the nodes here, but rather a subgraphID that's registered in the subgraph tracker.

Merging them would require adding some union-find thing to the tracker, or doing the backtracking lookahead I mentioned in the PR description.

I don't think it's really worth the effort/complexity for no real upsides.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah ok, put a comment somewhere in case we ever decide reducing #barriers / merging these things becomes important, and leave it

id: subgraph_id,
params: input_param_exprs.clone(),
});
}
} else {
// If there are registers to which to attach, emit it as a barrier command.

// Create pytket operation, and add the subcircuit as hugr
Copy link
Contributor

Choose a reason for hiding this comment

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

I think payload is a reference to an external subgraphid here, not a hugr, right?

let args = MakeOperationArgs {
num_qubits: op_values.qubits.len(),
num_bits: op_values.bits.len(),
params: Cow::Borrowed(&input_param_exprs),
};
let mut pytket_op = make_tk1_operation(tket_json_rs::OpType::Barrier, args);
pytket_op.data = Some(serde_json::to_string(&payload).unwrap());

// Create pytket operation, and add the subcircuit as hugr
let args = MakeOperationArgs {
num_qubits: op_values.qubits.len(),
num_bits: op_values.bits.len(),
params: Cow::Borrowed(&input_param_exprs),
};
let mut pytket_op = make_tk1_operation(tket_json_rs::OpType::Barrier, args);
pytket_op.data = Some(serde_json::to_string(&payload).unwrap());
self.emit_command(pytket_op, &op_values.qubits, &op_values.bits, None);
}

self.emit_command(pytket_op, &op_values.qubits, &op_values.bits, None);
Ok(())
}

Expand Down
2 changes: 2 additions & 0 deletions tket/src/serialize/pytket/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ pub enum PytketEncodeError<N: HugrNode = hugr::Node> {
#[display("An unsupported subgraph has no qubits or bits to attach the barrier command to{}",
if params.is_empty() {"".to_string()} else {format!(" alongside its parameters [{}]", params.iter().join(", "))}
)]
#[deprecated(since = "0.16.1", note = "No longer emitted since 0.16.1")]
#[from(ignore)]
UnsupportedSubgraphHasNoRegisters {
/// Parameter inputs to the unsupported subgraph.
params: Vec<String>,
Expand Down
40 changes: 40 additions & 0 deletions tket/src/serialize/pytket/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,41 @@ fn circ_complex_param_type() -> Circuit {
hugr.into()
}

/// A program with an unsupported subgraph not associated to any qubit or bit.
/// <https://github.com/CQCL/tket2/issues/1294>
#[fixture]
fn circ_unsupported_subgraph_no_registers() -> Circuit {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test looks fine, but you should probably add one which (currently) requires multiple additional-subgraphs...challenge if you can come up with a case that can't be encoded as just one?

Copy link
Collaborator Author

@aborgna-q aborgna-q Dec 4, 2025

Choose a reason for hiding this comment

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

👍 Modifying this to follow the structure from my comment above.

let input_t = vec![];
let output_t = vec![rotation_type()];
let mut h = FunctionBuilder::new(
"unsupported_subgraph_no_registers",
Signature::new(input_t, output_t),
)
.unwrap();

// Declare a function to call.
let func = {
let call_input_t = vec![];
let call_output_t = vec![float64_type()];
h.module_root_builder()
.declare("func", Signature::new(call_input_t, call_output_t).into())
.unwrap()
};

// An unsupported call that'll require an opaque subgraph to encode.
let call = h.call(&func, &[], []).unwrap();
let [f] = call.outputs_arr();

// An operation that must be marked as unsupported, since it's input cannot be encoded.
let [rot] = h
.add_dataflow_op(RotationOp::from_halfturns_unchecked, [f])
.unwrap()
.outputs_arr();

let hugr = h.finish_hugr_with_outputs([rot]).unwrap();
hugr.into()
}

/// Check that all circuit ops have been translated to a native gate.
///
/// Panics if there are tk1 ops in the circuit.
Expand Down Expand Up @@ -1005,6 +1040,11 @@ fn fail_on_modified_hugr(circ_tk1_ops: Circuit) {
)]
#[case::output_parameter_wire(circ_output_parameter_wire(), 1, CircuitRoundtripTestConfig::Default)]
#[case::non_local(circ_non_local(), 2, CircuitRoundtripTestConfig::Default)]
#[case::unsupported_subgraph_no_registers(
circ_unsupported_subgraph_no_registers(),
1,
CircuitRoundtripTestConfig::Default
)]

fn encoded_circuit_roundtrip(
#[case] circ: Circuit,
Expand Down
Loading