Skip to content

Conversation

@lmondada
Copy link
Contributor

@lmondada lmondada commented Oct 27, 2025

Previously, Intervals were storing ResourceIds and Positions. This made them only valid for a specific ResourceScope. This data is no longer stored, so that Intervals are now valid independently of the choice of ResourceIds and Positions. See the docstring of Interval and the added test.

BREAKING CHANGE: most methods of resource::Interval now take a ResourceScope argument; Interval::singleton constructor renamed to Interval::new_singleton

@hugrbot
Copy link
Collaborator

hugrbot commented Oct 27, 2025

This PR contains breaking changes to the public Rust API.

cargo-semver-checks summary
    Building tket v0.16.0 (current)
     Built [  40.749s] (current)
   Parsing tket v0.16.0 (current)
    Parsed [   0.095s] (current)
  Building tket v0.16.0 (baseline)
     Built [  40.464s] (baseline)
   Parsing tket v0.16.0 (baseline)
    Parsed [   0.089s] (baseline)
  Checking tket v0.16.0 -> v0.16.0 (assume minor change)
   Checked [   0.079s] 159 checks: 158 pass, 1 fail, 0 warn, 41 skip

--- failure struct_missing: pub struct removed or renamed ---

Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.45.0/src/lints/struct_missing.ron

Failed in:
struct tket::resource::Interval, previously in file /home/runner/work/tket2/tket2/BASELINE_BRANCH/tket/src/resource/interval.rs:13

   Summary semver requires new major version: 1 major and 0 minor checks failed
  Finished [  82.809s] tket
  Building tket-qsystem v0.22.0 (current)
     Built [  42.015s] (current)
   Parsing tket-qsystem v0.22.0 (current)
    Parsed [   0.028s] (current)
  Building tket-qsystem v0.22.0 (baseline)
     Built [  42.351s] (baseline)
   Parsing tket-qsystem v0.22.0 (baseline)
    Parsed [   0.029s] (baseline)
  Checking tket-qsystem v0.22.0 -> v0.22.0 (assume minor change)
   Checked [   0.046s] 159 checks: 159 pass, 41 skip
   Summary no semver update required
  Finished [  87.067s] tket-qsystem

@codecov
Copy link

codecov bot commented Oct 27, 2025

Codecov Report

❌ Patch coverage is 91.82156% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.07%. Comparing base (ad16531) to head (8b2482c).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
tket/src/subcircuit/interval.rs 91.89% 11 Missing and 10 partials ⚠️
tket/src/resource/scope.rs 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1205      +/-   ##
==========================================
+ Coverage   78.73%   79.07%   +0.34%     
==========================================
  Files         159      159              
  Lines       20157    20290     +133     
  Branches    19055    19188     +133     
==========================================
+ Hits        15870    16045     +175     
+ Misses       3310     3260      -50     
- Partials      977      985       +8     
Flag Coverage Δ
python 92.65% <ø> (ø)
qis-compiler 68.40% <ø> (ø)
rust 78.72% <91.82%> (+0.36%) ⬆️

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 lmondada marked this pull request as ready for review October 27, 2025 10:06
@lmondada lmondada requested a review from a team as a code owner October 27, 2025 10:06
@lmondada lmondada requested review from doug-q and removed request for doug-q October 27, 2025 10:06
@lmondada
Copy link
Contributor Author

lmondada commented Oct 27, 2025

@mariagg-quantinuum Could you review this please? I cannot requests from you atm 🫠

@mariagg-quantinuum
Copy link
Contributor

mariagg-quantinuum commented Oct 29, 2025

Checked this PR, all seems to work and make sense to me 👍 @lmondada
also I think you might be able to set me as a reviewer now so I can tick "review done"

@lmondada
Copy link
Contributor Author

Cheers, and

I think you might be able to set me as a reviewer now so I can tick "review done"

yes indeed, I can 🎉

@lmondada lmondada requested a review from doug-q October 29, 2025 14:14
@lmondada
Copy link
Contributor Author

@doug-q this was reviewed by @mariagg-quantinuum, but as she's not an owner of the repo, we would still need your approval.

A second pair of eyes is always good anyways! Thanks in advance 😊

github-merge-queue bot pushed a commit that referenced this pull request Oct 30, 2025
I want to define a `Subcircuit` as a combination of `Interval`s on
resource paths (the parts with linear, non-copyable data), and ASTs of
fully classical, copyable data.

- PR #1205 was opened to refine the notion of `Interval`
- this PR creates the `CopyableExpressionAST` type that corresponds to
ASTs of classical data in hugr graphs.

---------

Co-authored-by: Alan Lawrence <[email protected]>
@lmondada
Copy link
Contributor Author

lmondada commented Nov 5, 2025

@doug-q could you please review this when you have a moment?

Copy link
Contributor

@doug-q doug-q left a comment

Choose a reason for hiding this comment

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

Nice

@doug-q doug-q added this pull request to the merge queue Nov 6, 2025
Merged via the queue into main with commit b270af8 Nov 6, 2025
24 checks passed
@doug-q doug-q deleted the lm/interval-pr branch November 6, 2025 09:32
@hugrbot hugrbot mentioned this pull request Nov 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants