feat: Add ResourceScope by lmondada · Pull Request #1052 · Quantinuum/tket2

lmondada · 2025-08-19T14:32:19Z

Alright, couldn't resist it. Here's a PR that moves a lot of the logic that I tried to put into portgraph into tket. I think it makes much more sense this way.

I'm interested to see what you think! (a lot of the diff is docs and snapshot tests, so don't worry :P)

codecov · 2025-08-19T14:39:31Z

Codecov Report

❌ Patch coverage is 80.53333% with 146 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.14%. Comparing base (695e8d1) to head (80aa67b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
tket/src/resource/scope.rs	84.48%	48 Missing and 15 partials ⚠️
tket/src/resource/interval.rs	73.33%	34 Missing and 2 partials ⚠️
tket/src/resource/types.rs	70.10%	29 Missing ⚠️
tket/src/resource/flow.rs	81.03%	10 Missing and 1 partial ⚠️
tket/src/resource.rs	85.71%	3 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1052      +/-   ##
==========================================
+ Coverage   79.09%   79.14%   +0.04%     
==========================================
  Files         115      120       +5     
  Lines       13397    14147     +750     
  Branches    12615    13365     +750     
==========================================
+ Hits        10597    11196     +599     
- Misses       2146     2274     +128     
- Partials      654      677      +23

Flag	Coverage Δ
python	`92.83% <ø> (ø)`
rust	`78.33% <80.53%> (+0.09%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

lmondada · 2025-08-19T18:43:34Z

Three things on my mind

could we make this share more code/structs with the CircuitBuilder, esp. CircuitUnit?
idem, with the pytket encoder/decoder
imo this is a candidate to replace the Circuit struct if we get rid of it 😅

acl-cqc

In general I quite like this - the approach seems entirely reasonable within TKET, and perhaps a bit odd/overly-specialized in portgraph :). Haven't done a detailed code review but a few thoughts mostly on docs here.

My main question is - wtf CopyableValueID? Do you really want/use it, or did you just think you had to put something in for nonlinear ports? If the latter, would this PR work/make sense if you stripped it out?

acl-cqc · 2025-08-22T14:43:37Z

tket/src/resource.rs

+//!   the circuit. Each resource has a unique [`ResourceId`] and operations on
+//!   the same resource are ordered by a [`Position`].
+//! - **Copyable values**: Regular values that can be copied and discarded
+//!   freely. Each is identified by a unique [`CopyableValueId`].


I guess I will find out, but might be good to say that these are different/not-resource-tracked/something here.

Specified that.

tket/src/resource/flow.rs

acl-cqc · 2025-08-22T14:48:00Z

tket/src/resource/flow.rs

+    /// The i-th entry is Some(resource_id) if the i-th port is a linear type,
+    /// None otherwise. Returns the resource IDs of the operation's outputs
+    /// in port order. Output resource IDs should be one of the input resource
+    /// IDs for resource-preserving operations, or None for new resources or


I'm inferring here that every None is a new resource-id exactly if (iff) it's linear

Yes, I've clarified that!

tket/src/resource/flow.rs

tket/src/resource/interval.rs

acl-cqc · 2025-08-22T15:05:33Z

tket/src/resource.rs

+//!
+//! - **Linear resources**: Non-copyable values that form resource paths through
+//!   the circuit. Each resource has a unique [`ResourceId`] and operations on
+//!   the same resource are ordered by a [`Position`].


Say something like ResourceFlow allows to determine how resources are passed through, discarded or created by an op (for linear ports)

Added, thank you!

acl-cqc · 2025-08-22T15:06:39Z

tket/src/resource/scope.rs

@@ -0,0 +1,596 @@
+//! Main ResourceScope implementation for tracking resources in HUGR subgraphs.


nit: Can you drop the first line and just run with "Provides the [ResourceScope] struct which...." or does clippy/etc. complain?

I've shorted it a bit. I think it's good to have one liner first for doc generation. In this case it's used for the summary of modules, e.g.

acl-cqc · 2025-08-22T15:09:38Z

tket/src/resource/scope.rs

+    ) -> impl Iterator<Item = H::Node> + '_ {
+        let mut curr_node = start_node;
+
+        iter::from_fn(move || {


nit: does iter::successors let you avoid the mut curr_node?

oh, super neat!! Did not know about it!

acl-cqc · 2025-08-22T15:10:34Z

tket/src/resource/scope.rs

+        // order.
+        for node in toposort_subgraph(&self.hugr, &self.subgraph, self.find_sources()) {
+            if self.hugr.get_optype(node).dataflow_signature().is_none() {
+                // ignore non-dataflow ops


Whilst I think this is fine, I suspect non-dataflow nodes - like "dataflow" ops with no inputs or outputs - would just end any flows (and not start any new flows!) ?

Yes that's true. As a result, nodes that are not dataflow ops will not have any opvalues assigned to their ports. However, any dataflow op downstream will assign fresh resource IDs if required on line 303:

self.assign_missing_op_values(node, &mut allocator);

Do you have a better behaviour in mind for non-dataflow ops?

No...certainly they feel like no-ops, and I think they are. I wonder whether the continue is necessary, will the skipped code actually do anything?

the skipped code would panic on non-dataflow nodes. I have changed that to ignore any non-dataflow nodes, is definitely "better" code practice :)

acl-cqc · 2025-08-22T15:11:15Z

tket/src/resource/scope.rs

+    }
+
+    /// All copyable values on the ports of `node` in the given direction.
+    pub fn get_copyable_values(


(I note this isn't used....)

I have removed CopyableValueID and am using Wire instead. You were right -- it's much better.

acl-cqc · 2025-08-22T15:17:13Z

I mean, if this went into Portgraph, it'd have to do so without any mention of linearity, so probably leaving out any (useful!) implementation of the ResourceFlow trait entirely; that would then have to be done in tket...

lmondada · 2025-08-25T06:35:17Z

I will address your comments one by one. Before that though, one question:

I could replace CopyableValueID with just Wire - would that resolve your qualms? That begs the question: should I just use the CircuitUnit Hugr type? It's isomorphic to the type I need, but I was wondering:

is it worth having a tket-owned type for this?
this will be exposed in the Python API. Is CircuitUnit a good name? Is OpValue better? Is there something else we should consider
Using CircuitUnit::Linear means resources are represented by a usize, rather than a distinct type index. That's not great -- but does it justify another type?

Of course, conversions between CircuitUnit and whatever we'd use here could be implemented easily. The advantage the same type in both cases would be that someone familiar with one of them immediately recognises the type in the other context...

lmondada · 2025-08-26T06:13:11Z

Having slept on it, I will try to use the CircuitUnit enum. Might want to wrap it into a tket-owned type...

EDIT: There is now a CircuitUnit type that is currently equivalent to hugr::CircuitUnit but might change in the future. Using a foreign type (i.e. hugr::CircuitUnit) for this API was awkward as I could not add methods to it. I chose to use the same name to highlight the similarities between the types, but if you find that confusing please feel free to suggest other names for it :)

lmondada · 2025-08-26T07:21:16Z

Note: I've had to change trait ResourceFlow to be parametrised on H: HugrView. The reason for this is that I have generalised the fn map_resources function from taking an OpType to taking a Node and HugrView:

    fn map_resources(
        &self,
        node: H::Node,
        hugr: &H,
        inputs: &[Option<ResourceId>],
    ) -> Result<Vec<Option<ResourceId>>, UnsupportedOp>;

This was required to implement ResourceFlow e.g. on Call nodes, where the flow does not just depend on the optype, but on the function definition that is linked in.

I chose ResourceFlow<H> rather than impl HugrView in the function definition because the former trait is dyn-compatible, the latter is not.

acl-cqc · 2025-08-29T09:13:32Z

Yeah so I feel the Hugr CircuitUnit::Linear is not great, I think that usize refers to the ith among the linear inputs of the region (otherwise a Wire(inp,i), where inp is the region's Input node, could be used instead); but this is not terribly clear and there are many opportunities for getting the two variants confused (e.g. CircuitUnit::Copyable of a Wire that's linear, ....). Your CircuitUnit-containing-a-region is much better here, I think.

acl-cqc

Looking good! I suspect I won't make any more comments but I'll send now just to give myself time to think ;-). Or @tatiana-s do you want to take a look?

tket/src/resource.rs

tket/src/resource/flow.rs

tket/src/resource/scope.rs

acl-cqc · 2025-08-29T13:26:25Z

tket/src/resource/scope.rs

+        // order.
+        for node in toposort_subgraph(&self.hugr, &self.subgraph, self.find_sources()) {
+            if self.hugr.get_optype(node).dataflow_signature().is_none() {
+                // ignore non-dataflow ops


No...certainly they feel like no-ops, and I think they are. I wonder whether the continue is necessary, will the skipped code actually do anything?

tket/src/resource/scope.rs

tket/src/resource/types.rs

acl-cqc

Great stuff Luca, thanks for all the changes.

These are all 1-liner "nits" but there are a couple of wrong comments etc.

acl-cqc · 2025-09-02T16:59:50Z

tket/src/resource.rs

+//! introduces the notion of "Resource" to extend the lifetime of a linear value
+//! over multiple ops.
+//!
+//! If a linear value appears both in an op's input and output, we say that it


IIUC, this means that: a linear value is called "resource-preserving" if it appears in op's input and output.

Suggested change

//! If a linear value appears both in an op's input and output, we say that it

//! If a linear value appears both in an op's input and output, we say that the op "preserves" that value.

(or preserves that resource; perhaps "if a linear value appears in both input and output of an op")

It's now longer but more precise:

Every linear value is associated with a resource. If a linear value passed as input to an op is also returned by the op as an output, then both input and output values are associated with the same resource. We say that the op "preserves" the resource.

acl-cqc · 2025-09-02T17:03:09Z

tket/src/resource/flow.rs

+    ) -> Result<Vec<Option<ResourceId>>, UnsupportedOp>;
+
+    /// Convert this ResourceFlow into a boxed trait object.
+    fn into_boxed<'a>(self) -> Box<dyn 'a + ResourceFlow<H>>


Suggested change

fn into_boxed<'a>(self) -> Box<dyn 'a + ResourceFlow<H>>

fn into_boxed<'a>(self) -> Box<dyn 'a + Self>

Maybe Box<dyn '_ + Self works without the <'a> ?

I've tried, the compiler does not like it :(. I cannot replace ResourceFlow<H> with Self

expected trait, found self type Self

and cannot make the lifetime implicit either.

acl-cqc · 2025-09-02T17:05:02Z

tket/src/resource/flow.rs

+/// This implementation considers that an operation is resource-preserving if
+/// for all port indices i, either
+///  - the i-th input and i-th output are both linear and are of the same type,
+///  - or the i-th input and i-th output are both copyable, or one is copyable


Suggested change

/// - or the i-th input and i-th output are both copyable, or one is copyable

/// - or the i-th input and i-th output are both copyable,

/// - or one is copyable and the other does not exist

tket/src/resource/scope.rs

acl-cqc · 2025-09-02T17:14:23Z

tket/src/resource/scope.rs

+
+/// Configuration for a ResourceScope.
+pub struct ResourceScopeConfig<'a, H: HugrView> {
+    flows: Vec<Box<dyn 'a + ResourceFlow<H>>>,


Might be worth documenting that these will be tried in order (it wasn't obvious to me that the second would only be used if the first failed)

acl-cqc · 2025-09-02T17:19:27Z

tket/src/resource/scope.rs

+                    CircuitUnit::sentinel(),
+                    &signature,
+                ))
+                .into()


The .into adds the Some here? I think it might be clearer to do Some(...) explicitly...

acl-cqc · 2025-09-02T17:21:22Z

tket/src/resource/types.rs

+    }
+
+    /// Increment the position by 1.
+    pub fn increment(&self) -> Self {


You might want add_int(&self, usize) -> Self

I'll leave this to a minimum for the time being, it's always easy to add more if needed.

acl-cqc · 2025-09-02T17:22:20Z

tket/src/resource/types.rs

+/// A value associated with a dataflow port, identified either by a resource ID
+/// (for linear values) or by its wire (for copyable values).
+///
+/// This can currently be converted to and from [`hugr::CircuitUnit`], but


Remove this comment now (I think conversions in both directions are gone)

acl-cqc · 2025-09-02T17:24:09Z

tket/src/resource/types.rs

+    }
+
+    /// Get the underlying usize value of this ResourceId.
+    pub fn as_usize(self) -> usize {


I note this isn't used!! I guess it's reasonable to have it for indexing into an array etc.

Yes pretty sure I need it in one of my many local branches 😅

acl-cqc · 2025-09-02T17:25:20Z

tket/src/resource/types.rs

+    }
+}
+
+impl Default for ResourceAllocator {


I think you'll get this if you just derive Default, one could even argue there's no need for ::new()

Yes that's true. Removed! I'll define new in terms of Default.

## 🤖 New release * `tket`: 0.14.0 -> 0.15.0 (✓ API compatible changes) * `tket-qsystem`: 0.20.1 -> 0.21.0 (✓ API compatible changes) <details><summary>Changelog</summary> ## `tket` <blockquote> ## [0.15.0](tket-v0.14.0...tket-v0.15.0) - 2025-09-15 ### Bug Fixes - [**breaking**] Fix rotation -> float param type conversion ([#1061](#1061)) - Pytket barrier operations not being decoded ([#1069](#1069)) - Always load parameter expressions as half turns in the decoder ([#1083](#1083)) - Move attribute to come after all the cases. ([#1112](#1112)) ### New Features - Capture pytket's output permutation explicitly in the hugr connectivity ([#1075](#1075)) - Add ResourceScope ([#1052](#1052)) - [**breaking**] Remove unnecessary Arc from PytketDecoder method ([#1114](#1114)) - [**breaking**] Remove deprecated definitions ([#1113](#1113)) </blockquote> ## `tket-qsystem` <blockquote> ## [0.21.0](tket-qsystem-v0.20.1...tket-qsystem-v0.21.0) - 2025-09-15 ### Bug Fixes - [**breaking**] Fix rotation -> float param type conversion ([#1061](#1061)) - Pytket barrier operations not being decoded ([#1069](#1069)) - *(qystem)* fix angle bug in CZ decomposition ([#1080](#1080)) - Always load parameter expressions as half turns in the decoder ([#1083](#1083)) ### New Features - Add a `borrow_array` type replacement pass ([#975](#975)) - Add gpu module ([#1090](#1090)) - [**breaking**] Remove unnecessary Arc from PytketDecoder method ([#1114](#1114)) - [**breaking**] Remove deprecated definitions ([#1113](#1113)) ### Refactor - [**breaking**] Factor out wasm extension code into compute module ([#1089](#1089)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). --------- Co-authored-by: Agustín Borgna <121866228+aborgna-q@users.noreply.github.com> Co-authored-by: Alec Edgington <54802828+cqc-alec@users.noreply.github.com>

lmondada requested a review from aborgna-q August 19, 2025 14:32

lmondada force-pushed the lm/resource-scope branch from e58de1d to 57a2f0a Compare August 19, 2025 14:34

feat: Add ResourceScope

6b94a7d

lmondada force-pushed the lm/resource-scope branch from 57a2f0a to 6b94a7d Compare August 19, 2025 14:36

lmondada marked this pull request as ready for review August 19, 2025 14:36

lmondada requested a review from a team as a code owner August 19, 2025 14:36

This was referenced Aug 20, 2025

feat!: Replace Subcircuit // take 2 #1054

Closed

Remove Circuit struct #1060

Open

lmondada added 2 commits August 22, 2025 13:55

Merge branch 'main' into lm/resource-scope

3c86b95

Make position a node property

b07590e

lmondada requested review from acl-cqc and tatiana-s and removed request for aborgna-q August 22, 2025 12:14

acl-cqc reviewed Aug 22, 2025

View reviewed changes

fixes to resource flow

2942c7a

lmondada added 4 commits August 26, 2025 08:18

address comments

faea67d

rename OpValue -> CircuitUnit, remove CopyableValueID

c88549a

Merge branch 'main' into lm/resource-scope

6881abd

fix snapshots

78fbc35

lmondada requested a review from acl-cqc August 26, 2025 07:21

lmondada and others added 2 commits August 28, 2025 09:14

Merge branch 'main' into lm/resource-scope

d12af0d

expose CircuitUnit publicly

8e32c4c

acl-cqc reviewed Aug 29, 2025

View reviewed changes

address comments

6f2541c

lmondada requested a review from acl-cqc September 2, 2025 12:42

acl-cqc approved these changes Sep 2, 2025

View reviewed changes

lmondada added 3 commits September 3, 2025 19:12

Support empty scope

6f4b5f6

address comments

c858272

Merge branch 'main' into lm/resource-scope

80aa67b

lmondada added this pull request to the merge queue Sep 3, 2025

Merged via the queue into main with commit 64412df Sep 3, 2025
20 checks passed

lmondada deleted the lm/resource-scope branch September 3, 2025 17:53

hugrbot mentioned this pull request Sep 3, 2025

chore: release #1064

Closed

lmondada mentioned this pull request Sep 5, 2025

feat!: Add NodeType trait as hugr base trait Quantinuum/hugr#2541

Closed

hugrbot mentioned this pull request Sep 15, 2025

chore: release #1115

Merged

hugrbot mentioned this pull request Sep 16, 2025

chore: release #1119

Merged

		@@ -0,0 +1,596 @@
		//! Main ResourceScope implementation for tracking resources in HUGR subgraphs.

	//! If a linear value appears both in an op's input and output, we say that it
	//! If a linear value appears both in an op's input and output, we say that the op "preserves" that value.

	fn into_boxed<'a>(self) -> Box<dyn 'a + ResourceFlow<H>>
	fn into_boxed<'a>(self) -> Box<dyn 'a + Self>

	/// - or the i-th input and i-th output are both copyable, or one is copyable
	/// - or the i-th input and i-th output are both copyable,
	/// - or one is copyable and the other does not exist

Conversation

lmondada commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lmondada commented Aug 19, 2025

Uh oh!

acl-cqc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acl-cqc commented Aug 22, 2025

Uh oh!

lmondada commented Aug 25, 2025

Uh oh!

lmondada commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lmondada commented Aug 26, 2025

Uh oh!

acl-cqc commented Aug 29, 2025

Uh oh!

acl-cqc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acl-cqc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lmondada commented Aug 19, 2025 •

edited

Loading

codecov bot commented Aug 19, 2025 •

edited

Loading

lmondada commented Aug 26, 2025 •

edited

Loading

lmondada Sep 3, 2025 •

edited

Loading