Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add CircuitDiff #773

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat: Add CircuitDiff #773

wants to merge 6 commits into from

Conversation

lmondada
Copy link
Contributor

I'm introducing CircuitDiffs, for persistent graph rewriting! Once done, this will speed up badger-like workflows significantly (see below).

CircuitDiffs, which could be renamed to CircuitPatches if you prefer, store a subgraph for each of their parent diffs, along with the new replacement graph to rewrite the subgraphs to. Currently, diffs with no parents are created from a circuit, whilst other diffs are created by applying a SimpleReplacement onto a parent diff. This means that right now, only diffs with a single parent can be constructed (see below)

How speedup?

Given rewrites A, B and C with disjoint support, instead of having to consider individual graphs with respectively rewrites A, B, C, AB, BC, AC and ABC applied, the three rewrites can be applied independently and there will only be CircuitDiff for A, B and C. At pattern matching time, the matcher will have to consider the various combinations that are possible, but only within the local region that matches overlap with.

What's next

  • Implement a CircuitHistory: a set of diffs that are compatible with each other (i.e. there are no conflicting rewrites). Then rewrites can be applied to histories, which will create diffs with multiple parents. I will try to implement HugrView on these
  • Dominator trees and hashing: once we have a dag of diffs, I will keep pointers to the dominator of each, diff, this will make checking for compatible rewrites faster. We can then also keep the hashes of all dominated diffs in the dominator, to exclude create the same diffs multiple times, without relying on global hashes

@lmondada lmondada requested a review from aborgna-q February 14, 2025 10:08
@lmondada lmondada requested a review from a team as a code owner February 14, 2025 10:08
@hugrbot
Copy link
Collaborator

hugrbot commented Feb 14, 2025

This PR contains breaking changes to the public Rust API.
Please deprecate the old API instead (if possible), or mark the PR with a ! to indicate a breaking change.

cargo-semver-checks summary

Copy link

codecov bot commented Feb 14, 2025

Codecov Report

Attention: Patch coverage is 87.22359% with 52 lines in your changes missing coverage. Please review.

Project coverage is 82.69%. Comparing base (2cff11b) to head (6c8ffb0).

Files with missing lines Patch % Lines
tket2/src/diff/replacement.rs 90.10% 2 Missing and 25 partials ⚠️
tket2/src/diff.rs 80.18% 20 Missing and 1 partial ⚠️
tket2/src/circuit/hash.rs 85.71% 0 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #773      +/-   ##
==========================================
+ Coverage   82.46%   82.69%   +0.23%     
==========================================
  Files          65       67       +2     
  Lines        7979     8386     +407     
  Branches     7717     8124     +407     
==========================================
+ Hits         6580     6935     +355     
- Misses       1002     1024      +22     
- Partials      397      427      +30     
Flag Coverage Δ
python 82.06% <ø> (ø)
rust 82.71% <87.22%> (+0.23%) ⬆️

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.

@lmondada
Copy link
Contributor Author

This PR contains breaking changes to the public Rust API. Please deprecate the old API instead (if possible), or mark the PR with a ! to indicate a breaking change.

cargo-semver-checks summary

I've double checked, there are no breaking changes once commit f109186 is reverted. But it will require hugr#1920 to be merged, along with a hugr release.

@aborgna-q
Copy link
Collaborator

Fyi the semver-checks failure is due to it ignoring any cargo patches, so it tries to compile with hugr = "0.14.3" instead.

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.

3 participants