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

fix: Inconsistent behaviour in SiblingSubgraph::from_nodes #2011

Merged
merged 1 commit into from
Mar 21, 2025

Conversation

aborgna-q
Copy link
Collaborator

SiblingSubgraph::from_nodes and from_node have different definitions.

The former takes a set of nodes and defines a subgraph where the inputs and outputs correspond to all edges connecting the chosen nodes to external ones.
When a node has a disconnected port, it is ignored.

The latter does not check edges. It takes a single node and creates a subgraph using the dataflow signature of the operation.

Both definitions are valid and useful on their own, when calling from_nodes with a single-element vector we redirected the call to from_node, resulting in different port definitions.

This PR fixes that.

@aborgna-q aborgna-q added the bug Something isn't working label Mar 20, 2025
@aborgna-q aborgna-q requested a review from a team as a code owner March 20, 2025 17:09
@aborgna-q aborgna-q requested a review from zrho March 20, 2025 17:09
Copy link

codecov bot commented Mar 20, 2025

Codecov Report

Attention: Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.83%. Comparing base (f3dd145) to head (bad40dc).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
hugr-core/src/hugr/views/sibling_subgraph.rs 96.77% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2011   +/-   ##
=======================================
  Coverage   83.82%   83.83%           
=======================================
  Files         210      210           
  Lines       39645    39669   +24     
  Branches    36315    36339   +24     
=======================================
+ Hits        33232    33256   +24     
- Misses       4537     4538    +1     
+ Partials     1876     1875    -1     
Flag Coverage Δ
python 92.01% <ø> (ø)
rust 83.08% <96.77%> (+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.

Copy link

codspeed-hq bot commented Mar 20, 2025

CodSpeed Performance Report

Merging #2011 will not alter performance

Comparing ab/sibling-subgraph-from-node (bad40dc) with main (f3dd145)

Summary

✅ 21 untouched benchmarks

@aborgna-q aborgna-q force-pushed the ab/sibling-subgraph-from-node branch from b3dc6a5 to bad40dc Compare March 20, 2025 17:26
@aborgna-q aborgna-q requested a review from croyzor March 21, 2025 15:07
Copy link
Collaborator

@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, thank you

@aborgna-q aborgna-q added this pull request to the merge queue Mar 21, 2025
Merged via the queue into main with commit c8c88cd Mar 21, 2025
25 checks passed
@aborgna-q aborgna-q deleted the ab/sibling-subgraph-from-node branch March 21, 2025 15:17
@hugrbot hugrbot mentioned this pull request Mar 21, 2025
github-merge-queue bot pushed a commit that referenced this pull request Mar 21, 2025
## 🤖 New release

* `hugr-core`: 0.15.1 -> 0.15.2 (✓ API compatible changes)
* `hugr-llvm`: 0.15.1 -> 0.15.2
* `hugr-passes`: 0.15.1 -> 0.15.2
* `hugr`: 0.15.1 -> 0.15.2 (✓ API compatible changes)
* `hugr-cli`: 0.15.1 -> 0.15.2

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

## `hugr-core`

<blockquote>

##
[0.15.2](hugr-core-v0.15.1...hugr-core-v0.15.2)
- 2025-03-21

### Bug Fixes

- Don't enable envelope compression by default (yet)
([#2014](#2014))
- Inconsistent behaviour in `SiblingSubgraph::from_nodes`
([#2011](#2011))
</blockquote>

## `hugr-llvm`

<blockquote>

##
[0.15.1](hugr-llvm-v0.15.0...hugr-llvm-v0.15.1)
- 2025-03-21

### Bug Fixes

- Remove return from val_or_panic
([#1999](#1999))

### New Features

- add exit operation to prelude
([#2008](#2008))
- Add llvm codegen for collections.static_array
([#2003](#2003))
</blockquote>

## `hugr-passes`

<blockquote>

##
[0.15.1](hugr-passes-v0.15.0...hugr-passes-v0.15.1)
- 2025-03-21

### Bug Fixes

- correct `CallIndirect` tag from `FnCall` to `DataflowChild`
([#2006](#2006))
</blockquote>

## `hugr`

<blockquote>

##
[0.15.2](hugr-v0.15.1...hugr-v0.15.2)
- 2025-03-21

### Bug Fixes

- Don't enable envelope compression by default (yet)
([#2014](#2014))
- Inconsistent behaviour in `SiblingSubgraph::from_nodes`
([#2011](#2011))
</blockquote>

## `hugr-cli`

<blockquote>

##
[0.15.1](hugr-cli-v0.15.0...hugr-cli-v0.15.1)
- 2025-03-21

### New Features

- *(hugr-cli)* Nicer error when passing a non-envelope file
([#2007](#2007))
</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
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants