Skip to content

Conversation

@aborgna-q
Copy link
Collaborator

@aborgna-q aborgna-q commented May 2, 2025

Adds the concept of "entrypoint" to the HUGRs, detaching the "main node of interest" from the hierarchy root.

Now hugrs will always have a Module at their root (appropriately called HugrView::module_root), and any manipulation is mostly focused around the entrypoint instead.

Closes #2029
Closes #2010

before:

graph LR

subgraph 0 ["(0) DFG"]

direction LR

1["(1) Input"]

2["(2) Output"]

3["(3) test.quantum.CX"]

4["(4) test.quantum.CX"]

1--"0:0<br>qubit"-->3

1--"1:1<br>qubit"-->3

3--"0:1<br>qubit"-->4

3--"1:0<br>qubit"-->4

3-."2:2".->4

4--"0:0<br>qubit"-->2

4--"1:1<br>qubit"-->2

end
Loading

after:

graph LR

subgraph 0 ["(0) Module"]

direction LR

subgraph 1 ["(1) FuncDefn: #quot;main#quot;"]

direction LR

2["(2) Input"]

3["(3) Output"]

subgraph 4 ["(4) [**DFG**]"]

direction LR

style 4 stroke:#832561,stroke-width:3px

5["(5) Input"]

6["(6) Output"]

7["(7) test.quantum.CX"]

8["(8) test.quantum.CX"]

5--"0:0<br>qubit"-->7

5--"1:1<br>qubit"-->7

7--"0:1<br>qubit"-->8

7--"1:0<br>qubit"-->8

7-."2:2".->8

8--"0:0<br>qubit"-->6

8--"1:1<br>qubit"-->6

end

2--"0:0<br>qubit"-->4

2--"1:1<br>qubit"-->4

4--"0:0<br>qubit"-->3

4--"1:1<br>qubit"-->3

end

end
Loading

How does this affect...

...HugrView?

::root has been split into module_root and entrypoint. In general, you'll want to use the latter (as described in the docs).
nodes() is still defined, but now there's also descendants(node) and entry_descendants(), which should be preferred.

...builders?

The change should be transparent for hugr building operations. When you start an specific builder or call Hugr::new(op) we add additional nodes as necessary so that the new entrypoint is correctly defined inside a module;

  • If op is a module, we just leave it at the top.
  • If op can be defined in a module, we put it below the root.
  • If op is a dataflow operation, we define a "main" function with the same signature and put it there.
  • More exotic operations are not allowed, you'll need to build the container yourself and change the entrypoint afterwards.

...SiblingGraph, SiblingMut, and DescendantsGraph?

The structs are no more. Instead, HugrView has two new methods:

  • with_entrypoint(&self, Node) -> Rerooted<&Self>
  • with_entrypoint_mut(&mut self, Node) -> Rerooted<&mut Self>
    The new wrapper implements HugrMut, so it can be used transparently.

The main benefit of this is that Rerooted still contains all the nodes that are not descendants from the entrypoint. So external edges are still well-defined, and rewrite operations may still put look at the root module and define things there if needed.

...serialization?

The hugr json now has an entrypoint field. If missing, we assume it's the root.
For backwards compatibility, if the serialized root is not a module we will transparently wrap it as described before so older jsons will continue working without modifications.

hugr-module encoding is left as a TODO

...packages and envelopes?

Now every HUGR can be put in a package without modifications!
Ideally we'll now be able to remove the hugr <-> json methods and only use envelopes. (That's a TODO).

...mermaid/dot renders?

The entrypoint node is now highlighted in purple, and its title shown in bold between brackets. See the example above.

...passes?

Things should work as normal. New passes should look at the entrypoint and its descendants when looking for things to rewrite, instead of the whole hugr.

TODOs not in this PR:

BREAKING CHANGE: Hugrs now have an entrypoint node.
BREAKING CHANGE: Removed SiblingGraph / SiblingMut / DescendantsGraph

@aborgna-q aborgna-q requested a review from a team as a code owner May 2, 2025 13:52
@aborgna-q aborgna-q requested a review from ss2165 May 2, 2025 13:52
@codecov
Copy link

codecov bot commented May 2, 2025

Codecov Report

Attention: Patch coverage is 83.13397% with 141 lines in your changes missing coverage. Please review.

Project coverage is 82.10%. Comparing base (63c317b) to head (038ebcf).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
hugr-core/src/hugr/views/rerooted.rs 60.86% 54 Missing ⚠️
hugr-core/src/hugr/views.rs 85.10% 14 Missing ⚠️
hugr-core/src/hugr/patch.rs 0.00% 12 Missing ⚠️
hugr-core/src/builder/cfg.rs 76.92% 1 Missing and 8 partials ⚠️
hugr-core/src/hugr/patch/replace.rs 76.00% 6 Missing ⚠️
hugr-core/src/hugr/views/render.rs 70.00% 6 Missing ⚠️
hugr-core/src/builder/conditional.rs 66.66% 0 Missing and 5 partials ⚠️
hugr-core/src/hugr/internal.rs 54.54% 5 Missing ⚠️
hugr-core/src/hugr.rs 96.42% 0 Missing and 3 partials ⚠️
hugr-passes/src/nest_cfgs.rs 85.00% 3 Missing ⚠️
... and 18 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2147      +/-   ##
==========================================
- Coverage   82.17%   82.10%   -0.08%     
==========================================
  Files         228      227       -1     
  Lines       39976    39795     -181     
  Branches    36075    35894     -181     
==========================================
- Hits        32852    32672     -180     
- Misses       5293     5301       +8     
+ Partials     1831     1822       -9     
Flag Coverage Δ
python 85.64% <ø> (ø)
rust 81.71% <83.13%> (-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.

@codspeed-hq
Copy link

codspeed-hq bot commented May 2, 2025

CodSpeed Performance Report

Merging #2147 will degrade performances by 23.38%

Comparing ab/entrypoints (038ebcf) with main (63c317b)

Summary

❌ 2 regressions
✅ 19 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
simple_dfg 42.6 µs 55.6 µs -23.38%
simple_cfg_serialize/json 222.1 µs 264.6 µs -16.08%

@hugrbot
Copy link
Collaborator

hugrbot commented May 2, 2025

This PR contains breaking changes to the public Rust API.

cargo-semver-checks summary

--- failure constructible_struct_adds_field: externally-constructible struct adds field ---

Description:
A pub struct constructible with a struct literal has a new pub field. Existing struct literals must be updated to include the new field.
      ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.41.0/src/lints/constructible_struct_adds_field.ron

Failed in:
field InsertionResult.inserted_entrypoint in /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/hugr/hugrmut.rs:259

--- failure enum_missing: pub enum removed or renamed ---

Description:
A publicly-visible enum cannot be imported by its prior path. A `pub use` may have been removed, or the enum 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.41.0/src/lints/enum_missing.ron

Failed in:
enum hugr_core::package::PackageError, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/package.rs:382

--- failure enum_no_repr_variant_discriminant_changed: enum variant had its discriminant change value ---

Description:
The enum's variant had its discriminant value change. This breaks downstream code that used its value via a numeric cast like `as isize`.
      ref: https://doc.rust-lang.org/reference/items/enumerations.html#assigning-discriminant-values
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.41.0/src/lints/enum_no_repr_variant_discriminant_changed.ron

Failed in:
variant EnvelopeError::PackageEncoding 9 -> 8 in /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/envelope.rs:189
variant EnvelopeError::ModelImport 10 -> 9 in /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/envelope.rs:194
variant EnvelopeError::ModelRead 11 -> 10 in /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/envelope.rs:199
variant EnvelopeError::ModelWrite 12 -> 11 in /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/envelope.rs:204

--- failure enum_tuple_variant_changed_kind: An enum tuple variant changed kind ---

Description:
A public enum's exhaustive tuple variant has changed to a different kind of enum variant, breaking possible instantiations and patterns.
      ref: https://doc.rust-lang.org/reference/items/enumerations.html
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.41.0/src/lints/enum_tuple_variant_changed_kind.ron

Failed in:
variant InlineDFGError::NotDFG in /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/hugr/patch/inline_dfg.rs:18

--- failure enum_variant_missing: pub enum variant removed or renamed ---

Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It 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.41.0/src/lints/enum_variant_missing.ron

Failed in:
variant EnvelopeError::Package, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/envelope.rs:189
variant ValidationError::RootWithEdges, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/validate.rs:605
variant ValidationError::RootWithEdges, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/validate.rs:605
variant PackageEncodingError::Package, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/package.rs:408
variant InlineDFGError::NoParent, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/patch/inline_dfg.rs:21

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It 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.41.0/src/lints/inherent_method_missing.ron

Failed in:
Package::from_hugrs, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/package.rs:68

--- failure method_parameter_count_changed: pub method parameter count changed ---

Description:
A publicly-visible method now takes a different number of parameters.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#fn-change-arity
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.41.0/src/lints/method_parameter_count_changed.ron

Failed in:
hugr_core::hugr::Hugr::new now takes 0 parameters instead of 1, in /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/hugr.rs:93
hugr_core::Hugr::new now takes 0 parameters instead of 1, in /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/hugr.rs:93

--- failure module_missing: pub module removed or renamed ---

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

Failed in:
mod hugr_core::hugr::views::descendants, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views/descendants.rs:1
mod hugr_core::hugr::views::sibling, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views/sibling.rs:1

--- 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.41.0/src/lints/struct_missing.ron

Failed in:
struct hugr_core::hugr::views::descendants::DescendantsGraph, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views/descendants.rs:28
struct hugr_core::hugr::views::DescendantsGraph, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views/descendants.rs:28
struct hugr_core::hugr::views::sibling::SiblingMut, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views/sibling.rs:289
struct hugr_core::hugr::views::sibling::SiblingGraph, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views/sibling.rs:33
struct hugr_core::hugr::views::SiblingGraph, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views/sibling.rs:33

--- failure struct_pub_field_missing: pub struct's pub field removed or renamed ---

Description:
A publicly-visible struct has at least one public field that is no longer available under its prior name. It 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.41.0/src/lints/struct_pub_field_missing.ron

Failed in:
field new_root of struct InsertionResult, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/hugrmut.rs:226

--- failure trait_method_missing: pub trait method removed or renamed ---

Description:
A trait method is no longer callable, and may have been renamed or removed entirely.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#major-any-change-to-trait-item-signatures
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.41.0/src/lints/trait_method_missing.ron

Failed in:
method root of trait HugrView, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views.rs:41
method root_optype of trait HugrView, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views.rs:45
method root of trait HugrView, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views.rs:41
method root_optype of trait HugrView, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views.rs:45
method root of trait HugrView, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views.rs:41
method root_optype of trait HugrView, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views.rs:45
method set_root of trait HugrMutInternals, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/internal.rs:133

--- failure trait_missing: pub trait removed or renamed ---

Description:
A publicly-visible trait cannot be imported by its prior path. A `pub use` may have been removed, or the trait 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.41.0/src/lints/trait_missing.ron

Failed in:
trait hugr_core::hugr::views::ExtractHugr, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views.rs:423
trait hugr_core::hugr::views::HierarchyView, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views.rs:410

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It 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.41.0/src/lints/inherent_method_missing.ron

Failed in:
FatNode::try_new_hierarchy_view, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-llvm/src/utils/fat.rs:192

--- failure enum_tuple_variant_changed_kind: An enum tuple variant changed kind ---

Description:
A public enum's exhaustive tuple variant has changed to a different kind of enum variant, breaking possible instantiations and patterns.
      ref: https://doc.rust-lang.org/reference/items/enumerations.html
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.41.0/src/lints/enum_tuple_variant_changed_kind.ron

Failed in:
variant ConstFoldError::InvalidEntryPoint in /home/runner/work/hugr/hugr/PR_BRANCH/hugr-passes/src/const_fold.rs:44

@aborgna-q aborgna-q force-pushed the release-rs-v0.16.0 branch from 56c110c to 9cd24ff Compare May 7, 2025 10:54
Base automatically changed from release-rs-v0.16.0 to main May 7, 2025 11:02
@aborgna-q aborgna-q force-pushed the ab/entrypoints branch 2 times, most recently from 04a19d3 to 851e396 Compare May 7, 2025 17:36
Copy link
Member

@ss2165 ss2165 left a comment

Choose a reason for hiding this comment

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

Very happy to see so much red, and more readable code all-round.

the new InsertCut patch that just merged to main will need fixing on the rebase

@aborgna-q aborgna-q requested a review from ss2165 May 8, 2025 11:31
Copy link
Member

@ss2165 ss2165 left a comment

Choose a reason for hiding this comment

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

🚀

@aborgna-q aborgna-q added this pull request to the merge queue May 8, 2025
Merged via the queue into main with commit 8e36cdc May 8, 2025
26 of 27 checks passed
@aborgna-q aborgna-q deleted the ab/entrypoints branch May 8, 2025 12:30
github-merge-queue bot pushed a commit that referenced this pull request May 16, 2025
Adds entrypoint definitions to `hugr-py`. Has similar caveats to #2147.

The main issue here was builders that initialized a hugr for a dataflow
op without knowing its output types. The automatic machinery that wraps
the op in a function definition needs to connect the outputs to the new
function's output once we know the types, so I had to add some extra
machinery to `DataflowOp` to signal back to the hugr that it should
connect things.
This feels a bit hacky, perhaps there's a cleaner way to do it...

Blocked by #2147.

BREAKING CHANGE: Hugrs now define an `entrypoint` in addition to a
module root.
This was referenced May 29, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jun 13, 2025
…#2336)

This should have been part of #2256 but some confusion dates from #2147.

Making a callgraph for only the functions beneath the entrypoint makes
no sense - that's either all the functions (if the entrypoint is the
root) or at most one of them (if the entrypoint is one of the
funcdefns). Of course, we didn't test with any Hugrs that had a
non-module-root entrypoint.... so the sensible thing to do now, seems to
be for callgraph to ignore the entrypoint. (We can optimize it looking
for FuncDefns tho, they are now easy to find.)

Dead function removal allowed specifying starting points for analysis
(top-level FuncDefns) but only if the Hugr's entrypoint was the module.
There seems no good reason not to allow specifying module-level
FuncDefns even if the entrypoint is lower, and similarly, we should use
the entrypoint as one starting point (perhaps among many) for
analysis...

(One could rename `entry_points` used by analysis to "starting_points"
but that would be breaking)
ss2165 pushed a commit that referenced this pull request Jun 24, 2025
…#2336)

This should have been part of #2256 but some confusion dates from #2147.

Making a callgraph for only the functions beneath the entrypoint makes
no sense - that's either all the functions (if the entrypoint is the
root) or at most one of them (if the entrypoint is one of the
funcdefns). Of course, we didn't test with any Hugrs that had a
non-module-root entrypoint.... so the sensible thing to do now, seems to
be for callgraph to ignore the entrypoint. (We can optimize it looking
for FuncDefns tho, they are now easy to find.)

Dead function removal allowed specifying starting points for analysis
(top-level FuncDefns) but only if the Hugr's entrypoint was the module.
There seems no good reason not to allow specifying module-level
FuncDefns even if the entrypoint is lower, and similarly, we should use
the entrypoint as one starting point (perhaps among many) for
analysis...

(One could rename `entry_points` used by analysis to "starting_points"
but that would be breaking)
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.

HUGR root pointers refactor(hugr-core): SiblingGraph and DescendantsGraph do not work with parametric node types

4 participants