Skip to content

fix: Dataflow analysis produces unsound results on Hugrs with entrypoint #2255

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.

Sign up for GitHub

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

Jump to bottom
Merged
acl-cqc merged 5 commits into from
Jun 3, 2025
Merged

fix: Dataflow analysis produces unsound results on Hugrs with entrypoint #2255

acl-cqc merged 5 commits into from
Jun 3, 2025

Conversation

@acl-cqc
Copy link
Contributor

@acl-cqc acl-cqc commented May 26, 2025 •
edited
Loading

The bigger issue here is #2254, which this PR does not solve; it is a workaround.

However, it at least avoids unsoundness, by running the analysis on all nodes in the Hugr, rather than only those beneath the entrypoint. While this loses the ability to run analysis faster by restricting to within entry_descendants (that would be the full solution to #2254, not done here), it still

  • allows using the entrypoint to identify the starting point to which inputs are fed in [Machine::run]
  • provdes more accurate results than if we ran only within the subtree
  • reduces memory after the analysis is complete (by returning only filtered results)

@acl-cqc acl-cqc changed the title bugfix: unsound results for dataflow analysis for Hugrs with entrypoint fix: Dataflow analysis produces unsound results on Hugrs with entrypoint May 26, 2025
@codecov
Copy link

codecov bot commented May 26, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.17%. Comparing base (ead071b) to head (6174928).
Report is 3 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2255   +/-   ##
=======================================
  Coverage   82.17%   82.17%           
=======================================
  Files         239      239           
  Lines       43012    43016    +4     
  Branches    38924    38928    +4     
=======================================
+ Hits        35346    35350    +4     
  Misses       5675     5675           
  Partials     1991     1991
Flag Coverage Δ
python 85.34% <ø> (ø)
rust 81.84% <100.00%> (+<0.01%) ⬆️

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.

acl-cqc
acl-cqc commented May 26, 2025
View reviewed changes
hugr-passes/src/dataflow/test.rs
Some(pv_false())
);

// Do not provide a value on the second input (constant false in the whole hugr, above)
Copy link
Contributor Author

@acl-cqc acl-cqc May 26, 2025

Choose a reason for hiding this comment

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

This was not testing what was intended: not passing the second input, just makes Machine::run / Machine::prepopulate_inputs provide Top on the missing input, so there is no connection with (or reliance on the edge from) the graph outside the entrypoint. Hence, replacing the second input with a nonlocal edge.

@acl-cqc acl-cqc marked this pull request as ready for review May 26, 2025 16:12
@acl-cqc acl-cqc requested a review from a team as a code owner May 26, 2025 16:12
@acl-cqc acl-cqc requested a review from tatiana-s May 26, 2025 16:12
@acl-cqc acl-cqc mentioned this pull request May 26, 2025
Merged
@acl-cqc acl-cqc enabled auto-merge May 28, 2025 16:11
@ss2165 ss2165 requested a review from a team as a code owner May 29, 2025 12:43
@ss2165 ss2165 requested a review from aborgna-q May 29, 2025 12:43
@ss2165 ss2165 added this to the hugr-rs 0.20.x milestone May 30, 2025
@acl-cqc acl-cqc disabled auto-merge June 2, 2025 08:46
@acl-cqc acl-cqc added this pull request to the merge queue Jun 3, 2025
Merged via the queue into main with commit f8a7490 Jun 3, 2025
27 checks passed
@acl-cqc acl-cqc deleted the acl/dataflow_root branch June 3, 2025 08:59
@hugrbot hugrbot mentioned this pull request Jun 3, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jun 3, 2025
@hugrbot
chore: release v0.20.1 (#2273)
97466ce 
## 🤖 New release

* `hugr-model`: 0.20.0 -> 0.20.1
* `hugr-core`: 0.20.0 -> 0.20.1 (✓ API compatible changes)
* `hugr-llvm`: 0.20.0 -> 0.20.1 (✓ API compatible changes)
* `hugr-passes`: 0.20.0 -> 0.20.1 (✓ API compatible changes)
* `hugr`: 0.20.0 -> 0.20.1 (✓ API compatible changes)
* `hugr-cli`: 0.20.0 -> 0.20.1 (✓ API compatible changes)

<details><summary><i><b>Changelog</b></i></summary><p>

## `hugr-model`

<blockquote>

##
[0.20.0](hugr-model-v0.19.0...hugr-model-v0.20.0)
- 2025-05-14

### New Features

- [**breaking**] Mark all Error enums as non_exhaustive
([#2056](#2056))
- [**breaking**] Bump MSRV to 1.85
([#2136](#2136))
- Export and import entrypoints via metadata in `hugr-model`.
([#2172](#2172))
- Define text-model envelope formats
([#2188](#2188))
- Import CFG regions without adding an entry block.
([#2200](#2200))
- Symbol applications can leave out prefixes of wildcards.
([#2201](#2201))
</blockquote>

## `hugr-core`

<blockquote>

##
[0.20.1](hugr-core-v0.20.0...hugr-core-v0.20.1)
- 2025-06-03

### Bug Fixes

- check well-definedness of DFG wires in validate
([#2221](#2221))
- Check for order edges in SiblingSubgraph::from_node
([#2223](#2223))
- Make SumType::Unit(N) equal to SumType::General([(); N])
([#2250](#2250))
- canonicalize_nodes sometimes mangles the entrypoint
([#2263](#2263))

### New Features

- Add PersistentHugr ([#2080](#2080))
- Add `Type::used_extensions`
([#2224](#2224))
- Add boundary edge traversal in SimpleReplacement
([#2231](#2231))
- Add signature map function for DFGs
([#2239](#2239))
- PersistentHugr implements HugrView
([#2202](#2202))
- PersistentHugr Walker API
([#2168](#2168))
- Hugr::store_with_exts and auto-include in serde_as
([#2280](#2280))

### Refactor

- tidies/readability improvements to PersistentHugr
([#2251](#2251))
- Deprecate ValidationError::ExtensionError
([#2281](#2281))

### Testing

- Ignore miri errors in tests involving `assert_snapshot`
([#2261](#2261))
</blockquote>

## `hugr-llvm`

<blockquote>

##
[0.20.1](hugr-llvm-v0.20.0...hugr-llvm-v0.20.1)
- 2025-06-03

### Bug Fixes

- Make SumType::Unit(N) equal to SumType::General([(); N])
([#2250](#2250))

### Testing

- Add exec tests for widen op
([#2043](#2043))
</blockquote>

## `hugr-passes`

<blockquote>

##
[0.20.1](hugr-passes-v0.20.0...hugr-passes-v0.20.1)
- 2025-06-03

### Bug Fixes

- Dataflow analysis produces unsound results on Hugrs with entrypoint
([#2255](#2255))

### New Features

- LocalizeEdges pass ([#2237](#2237))
</blockquote>

## `hugr`

<blockquote>

##
[0.20.1](hugr-v0.20.0...hugr-v0.20.1)
- 2025-06-03

### Bug Fixes

- Dataflow analysis produces unsound results on Hugrs with entrypoint
([#2255](#2255))
- check well-definedness of DFG wires in validate
([#2221](#2221))
- Check for order edges in SiblingSubgraph::from_node
([#2223](#2223))
- Make SumType::Unit(N) equal to SumType::General([(); N])
([#2250](#2250))
- canonicalize_nodes sometimes mangles the entrypoint
([#2263](#2263))

### New Features

- LocalizeEdges pass ([#2237](#2237))
- Add PersistentHugr ([#2080](#2080))
- Add `Type::used_extensions`
([#2224](#2224))
- Add boundary edge traversal in SimpleReplacement
([#2231](#2231))
- Add signature map function for DFGs
([#2239](#2239))
- PersistentHugr implements HugrView
([#2202](#2202))
- PersistentHugr Walker API
([#2168](#2168))
- Hugr::store_with_exts and auto-include in serde_as
([#2280](#2280))

### Refactor

- tidies/readability improvements to PersistentHugr
([#2251](#2251))
- Deprecate ValidationError::ExtensionError
([#2281](#2281))

### Testing

- Ignore miri errors in tests involving `assert_snapshot`
([#2261](#2261))
</blockquote>

## `hugr-cli`

<blockquote>

##
[0.20.1](hugr-cli-v0.20.0...hugr-cli-v0.20.1)
- 2025-06-03

### New Features

- support external subcommands via PATH
([#1343](#1343))
([#2278](#2278))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tatiana-s tatiana-s tatiana-s approved these changes

@aborgna-q aborgna-q Awaiting requested review from aborgna-q aborgna-q is a code owner automatically assigned from Quantinuum/hugr

+1 more reviewer

@zrho zrho zrho approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

hugr-rs 0.20.x

Development

Successfully merging this pull request may close these issues.

5 participants

@acl-cqc @zrho @tatiana-s @ss2165