feat: Add HugrMut::insert(_view)_forest #2518

acl-cqc · 2025-08-12T12:21:26Z

New trait methods can insert several subtrees at once at different locations in target Hugr
trait-default-implement insert_hugr, insert_view and insert_subgraph using these

Technically I think this is breaking, any external impl HugrMut now has to provide these new methods, but we believe implementations of HugrMut outside of hugr-core to be sufficiently rare not to justify a major-version-number release, and rs-semver-checks does not seem to pick up on this.

…r/from_view

codecov · 2025-08-12T12:24:40Z

Codecov Report

❌ Patch coverage is 96.62921% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.76%. Comparing base (75fd767) to head (57e58e2).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
hugr-core/src/hugr/views/rerooted.rs	0.00%	2 Missing ⚠️
hugr-core/src/hugr/views/impls.rs	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2518      +/-   ##
==========================================
+ Coverage   82.66%   82.76%   +0.09%     
==========================================
  Files         252      252              
  Lines       46801    47065     +264     
  Branches    42330    42581     +251     
==========================================
+ Hits        38689    38952     +263     
+ Misses       6061     6055       -6     
- Partials     2051     2058       +7

Flag	Coverage Δ
python	`91.41% <ø> (+0.02%)`	⬆️
rust	`81.85% <96.62%> (+0.10%)`	⬆️

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.

hugr-core/src/hugr/hugrmut.rs

This reverts commit caecc43.

aborgna-q · 2025-08-19T13:43:43Z

hugr/benches/benchmarks/hugr.rs

+        // Note it would be better to use iter_batched to avoid cloning nodes/roots.
+        let nodes = insert.entry_descendants().chain([defn.node(), decl.node()]);
+        let roots = [
+            (insert.entrypoint(), h.entrypoint()),
+            (defn.node(), h.module_root()),
+            (decl.node(), h.module_root()),
+        ];
+        b.iter(|| black_box(h.insert_view_forest(&insert, nodes.clone(), roots.iter().cloned())))


You are modifying h on each iteration, so it becomes bigger and bigger each time!

Yes, but I don't think that should matter - we are not, e.g., copying h (or validating it) on each iteration; it's only if resizing nodes arrays etc. where this might hurt....and that should be amortized, right?

I can change it if you think this is important??

Each benchmark run should ideally have the same context, to avoid noise.
Re-using the hugr has implications in vector allocations, and cache hits/misses.

We can just use a Hugr::new() here.

hugr-core/src/hugr/hugrmut.rs

…_parents

acl-cqc · 2025-08-20T08:41:42Z

hugr-core/src/hugr/hugrmut.rs

+    #[display(
+        "Subtree rooted at {subtree} is already being copied as part of that rooted at {parent}"
+    )]
+    SubtreeAlreadyCopied {


Possibly this would be better called SubtreesNotDisjoint or SubtreesOverlap or something like that

aborgna-q

LGTM, just fix the benchmark before merging

acl-cqc · 2025-09-02T13:08:21Z

Ok after updating benchmarks and making the other factors the same on both sides, this comparison shows the improvement that could be gained by requiring the other_nodes iterator to be in order (and not requiring said iterator to be Clone): https://codspeed.io/CQCL/hugr/runs/compare/68b5d6779504c8fab797415c..68b6d66cc116c5dd4ccf13d0 as in commit e1a9674 (diff)

I think that's small enough that I'm merging.

## 🤖 New release * `hugr-model`: 0.22.2 -> 0.22.3 (✓ API compatible changes) * `hugr-core`: 0.22.2 -> 0.22.3 (✓ API compatible changes) * `hugr-llvm`: 0.22.2 -> 0.22.3 (✓ API compatible changes) * `hugr-passes`: 0.22.2 -> 0.22.3 (✓ API compatible changes) * `hugr-persistent`: 0.2.2 -> 0.2.3 (✓ API compatible changes) * `hugr`: 0.22.2 -> 0.22.3 (✓ API compatible changes) * `hugr-cli`: 0.22.2 -> 0.22.3 (✓ API compatible changes) <details><summary>Changelog</summary> ## `hugr-model` <blockquote> ## [0.22.2](hugr-model-v0.22.1...hugr-model-v0.22.2) - 2025-08-06 ### New Features - Type of constants in `core` `Term`s. ([#2411](#2411)) </blockquote> ## `hugr-core` <blockquote> ## [0.22.3](hugr-core-v0.22.2...hugr-core-v0.22.3) - 2025-09-11 ### Bug Fixes - SiblingSubgraph::try_from_nodes not including disconnected components ([#2549](#2549)) ### Documentation - Clarify docs for SiblingSubgraph::{inputs, outputs} ([#2508](#2508)) ### New Features - SiblingSubgraph supports function calls ([#2528](#2528)) - Add unchecked constructor for SiblingSubgraph ([#2526](#2526)) - Add HugrMut::insert(_view)_forest ([#2518](#2518)) - Add extend_inputs function for DFGs ([#2536](#2536)) - Loosen bound on Patch trait ([#2545](#2545)) </blockquote> ## `hugr-llvm` <blockquote> ## [0.22.2](hugr-llvm-v0.22.1...hugr-llvm-v0.22.2) - 2025-08-06 ### Bug Fixes - added public func getter for EmitFuncContext ([#2482](#2482)) - *(hugr-llvm)* Set llvm function linkage based on Visibility hugr node field ([#2502](#2502)) </blockquote> ## `hugr-passes` <blockquote> ## [0.22.3](hugr-passes-v0.22.2...hugr-passes-v0.22.3) - 2025-09-11 ### New Features - SiblingSubgraph supports function calls ([#2528](#2528)) </blockquote> ## `hugr-persistent` <blockquote> ## [0.2.3](hugr-persistent-v0.2.2...hugr-persistent-v0.2.3) - 2025-09-11 ### Documentation - Clarify docs for SiblingSubgraph::{inputs, outputs} ([#2508](#2508)) ### New Features - SiblingSubgraph supports function calls ([#2528](#2528)) </blockquote> ## `hugr` <blockquote> ## [0.22.3](hugr-v0.22.2...hugr-v0.22.3) - 2025-09-11 ### Bug Fixes - SiblingSubgraph::try_from_nodes not including disconnected components ([#2549](#2549)) ### Documentation - Clarify docs for SiblingSubgraph::{inputs, outputs} ([#2508](#2508)) ### New Features - SiblingSubgraph supports function calls ([#2528](#2528)) - Add unchecked constructor for SiblingSubgraph ([#2526](#2526)) - Add extend_inputs function for DFGs ([#2536](#2536)) - Loosen bound on Patch trait ([#2545](#2545)) - Add HugrMut::insert(_view)_forest ([#2518](#2518)) </blockquote> ## `hugr-cli` <blockquote> ## [0.22.3](hugr-cli-v0.22.2...hugr-cli-v0.22.3) - 2025-09-11 ### New Features - *(hugr-cli)* CliError::validate helper ([#2507](#2507)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/).

acl-cqc added 4 commits August 11, 2025 13:28

Add HugrMut::insert_forest / insert_view_forest, implement insert_hug…

1723b48

…r/from_view

insert_subgraph delegates to insert_forest (copying nodes list)

2e66542

errors; docs

02fea73

Tests

fdee944

acl-cqc added 2 commits August 12, 2025 14:59

fix test - edges removed rather than delocalized

0eaf3eb

Merge remote-tracking branch 'origin/main' into acl/insert_forest

05072b6

acl-cqc commented Aug 12, 2025

View reviewed changes

hugr-core/src/hugr/hugrmut.rs Outdated Show resolved Hide resolved

acl-cqc commented Aug 12, 2025

View reviewed changes

hugr-core/src/hugr/hugrmut.rs Show resolved Hide resolved

acl-cqc commented Aug 12, 2025

View reviewed changes

hugr-core/src/hugr/hugrmut.rs Outdated Show resolved Hide resolved

acl-cqc requested a review from aborgna-q August 12, 2025 14:50

acl-cqc marked this pull request as ready for review August 12, 2025 14:50

acl-cqc requested a review from a team as a code owner August 12, 2025 14:50

acl-cqc commented Aug 12, 2025

View reviewed changes

hugr-core/src/hugr/hugrmut.rs Show resolved Hide resolved

acl-cqc mentioned this pull request Aug 13, 2025

feat: add trait+funcs for linking Hugrs explicitly by Node #2521

Merged

More docs/links

f55c288

acl-cqc commented Aug 13, 2025

View reviewed changes

hugr-core/src/hugr/hugrmut.rs Outdated Show resolved Hide resolved

aborgna-q reviewed Aug 14, 2025

View reviewed changes

hugr-core/src/hugr/hugrmut.rs Outdated Show resolved Hide resolved

acl-cqc added 10 commits August 15, 2025 17:02

Optimize by turning HashMap into impl IntoIterator, avoid size_hint

10778ce

struct InsertedForest; use InsertForestResult alias more widely

826990d

N -> SN

f9eee44

Make DoubleCopy contain a named field

caecc43

Revert "Make DoubleCopy contain a named field"

bd12196

This reverts commit caecc43.

Use rstest::fixture

bdc0ff1

test tidy

58dccdd

add benchmark

b1c0d2a

lint benchmark and add to criterion group

88a4987

comments

cb92e0c

acl-cqc requested a review from aborgna-q August 17, 2025 21:28

acl-cqc force-pushed the acl/insert_forest branch from 1d23bc8 to cb92e0c Compare August 19, 2025 11:03

Better benchmarks via iter_batched

a73ce4f

aborgna-q reviewed Aug 19, 2025

View reviewed changes

acl-cqc added 3 commits August 19, 2025 15:04

Always convert to HashMap; update insert_forest_internal; roots->root…

481648e

…_parents

Do not require HashMap, so insert_view_forest can avoid building one

9a9c325

Separate errors: DoubleCopy => DuplicateNode + SubtreeAlreadyCopied

ec01c36

acl-cqc requested a review from aborgna-q August 19, 2025 16:08

acl-cqc commented Aug 20, 2025

View reviewed changes

acl-cqc added 4 commits August 20, 2025 09:46

docs

8426498

Builder docs for #2496

f8bdaa1

remove not-yet-existent doclink

17a8de4

Improve doc of dfg_calling_defn_decl

191119f

aborgna-q approved these changes Sep 1, 2025

View reviewed changes

acl-cqc added 2 commits September 1, 2025 18:13

insert_view_forest uses iter_batched and recreates host each time

685e19f

oops, fix borrow in bench

57e58e2

acl-cqc added this pull request to the merge queue Sep 2, 2025

Merged via the queue into main with commit 4b38008 Sep 2, 2025
26 checks passed

acl-cqc deleted the acl/insert_forest branch September 2, 2025 13:16

hugrbot mentioned this pull request Sep 1, 2025

chore: release #2509

Merged

acl-cqc mentioned this pull request Sep 17, 2025

insert_hugr is broken #2496

Closed

feat: Add HugrMut::insert(_view)_forest #2518

feat: Add HugrMut::insert(_view)_forest #2518

Uh oh!

Conversation

acl-cqc commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aborgna-q Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

acl-cqc Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aborgna-q Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

acl-cqc Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

aborgna-q left a comment

Choose a reason for hiding this comment

Uh oh!

acl-cqc commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

acl-cqc commented Aug 12, 2025 •

edited

Loading

codecov bot commented Aug 12, 2025 •

edited

Loading

acl-cqc Aug 19, 2025 •

edited

Loading

aborgna-q Sep 1, 2025 •

edited

Loading

acl-cqc commented Sep 2, 2025 •

edited

Loading