feat(hugr-py): automatically add state order edges for inter-graph edges #1165

ss2165 · 2024-06-05T16:38:08Z

uses a slightly dodgy -1 offset hack

hack currently means if you query the hugr for outgoing edges the state order edges are ignored.

codecov · 2024-06-05T16:45:53Z

Codecov Report

Attention: Patch coverage is 92.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 87.04%. Comparing base (6eb6d56) to head (286e420).

Files	Patch %	Lines
hugr-py/src/hugr/_dfg.py	85.71%	2 Missing ⚠️
hugr-py/src/hugr/_exceptions.py	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1165      +/-   ##
==========================================
+ Coverage   87.03%   87.04%   +0.01%     
==========================================
  Files          96       97       +1     
  Lines       19207    19246      +39     
  Branches    18398    18398              
==========================================
+ Hits        16716    16752      +36     
- Misses       1638     1641       +3     
  Partials      853      853

Flag	Coverage Δ
python	`89.38% <92.50%> (+0.14%)`	⬆️

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.

mark-koch

I think this will be sufficient for Guppy but note that non-local Dom edges from the spec are not covered by this PR. Should the builder handle those as well?

Anyways that can propbably wait for another PR

mark-koch · 2024-06-06T11:10:33Z

hugr-py/src/hugr/_dfg.py

+def _ancestral_sibling(h: Hugr, src: Node, tgt: Node) -> Node | None:
+    src_parent = h[src].parent
+
+    while (tgt_parent := h[tgt].parent) is not None:


You could get away with this:

Suggested change

while (tgt_parent := h[tgt].parent) is not None:

while tgt_parent := h[tgt].parent:

i prefer explicit checks, python truthiness is gross

mark-koch · 2024-06-06T11:12:36Z

hugr-py/src/hugr/_hugr.py

@@ -292,6 +292,7 @@ def _node_links(
            return
        # iterate over known offsets
        for offset in range(self.num_ports(node, direction)):
+            # TODO should this also look for -1 state order edges?


I'd prefer a second method to get the order links

just the order links or the order links along with the value?

ss2165 · 2024-06-06T11:20:53Z

Yeah this is not sufficient to handle Dom, my intent is to cover that with the CFG PR, because we can probably do some overloaded behaviour by detecting we are inside a basic block

🤖 I have created a release *beep* *boop* --- ## [0.3.0](hugr-py-v0.2.1...hugr-py-v0.3.0) (2024-07-03) ### ⚠ BREAKING CHANGES * * `add_child_op`(`_with_parent`), etc., gone; use `add_child_node`(`_with_parent`) with an (impl Into-)OpType. * `get_nodetype` gone - use `get_optype`. * `NodeType` gone - use `OpType` directly. * Various (Into<)Option<ExtensionSet> params removed from builder methods especially {cfg_,dfg_}builder. * `input_extensions` removed from serialization schema. * the Signature class is gone, but it's not clear how or why you might have been using it... * TailLoop node and associated builder functions now require specifying an ExtensionSet; extension/validate.rs deleted; some changes to Hugrs validated/rejected when the `extension_inference` feature flag is turned on * Type::validate takes extra bool (allow_rowvars); renamed {FunctionType, PolyFuncType}::(validate=>validate_var_len). ### Features * Allow "Row Variables" declared as List<Type> ([#804](#804)) ([3ea4834](3ea4834)) * **hugr-py:** add builders for Conditional and TailLoop ([#1210](#1210)) ([43569a4](43569a4)) * **hugr-py:** add CallIndirect, LoadFunction, Lift, Alias ([#1218](#1218)) ([db09193](db09193)), closes [#1213](#1213) * **hugr-py:** add values and constants ([#1203](#1203)) ([f7ea178](f7ea178)), closes [#1202](#1202) * **hugr-py:** automatically add state order edges for inter-graph edges ([#1165](#1165)) ([5da06e1](5da06e1)) * **hugr-py:** builder for function definition/declaration and call ([#1212](#1212)) ([af062ea](af062ea)) * **hugr-py:** builder ops separate from serialised ops ([#1140](#1140)) ([342eda3](342eda3)) * **hugr-py:** CFG builder ([#1192](#1192)) ([c5ea47f](c5ea47f)), closes [#1188](#1188) * **hugr-py:** define type hierarchy separate from serialized ([#1176](#1176)) ([10f4c42](10f4c42)) * **hugr-py:** only require input type annotations when building ([#1199](#1199)) ([2bb079f](2bb079f)) * **hugr-py:** python hugr builder ([#1098](#1098)) ([23408b5](23408b5)) * **hugr-py:** store children in node weight ([#1160](#1160)) ([1cdaeed](1cdaeed)), closes [#1159](#1159) * **hugr-py:** ToNode interface to treat builders as nodes ([#1193](#1193)) ([1da33e6](1da33e6)) * Validate Extensions using hierarchy, ignore input_extensions, RIP inference ([#1142](#1142)) ([8bec8e9](8bec8e9)) ### Bug Fixes * Add some validation for const nodes ([#1222](#1222)) ([c05edd3](c05edd3)) * **hugr-py:** more ruff lints + fix some typos ([#1246](#1246)) ([f158384](f158384)) * **py:** get rid of pydantic config deprecation warnings ([#1084](#1084)) ([52fcb9d](52fcb9d)) ### Documentation * **hugr-py:** add docs link to README ([#1259](#1259)) ([d2a9148](d2a9148)) * **hugr-py:** build and publish docs ([#1253](#1253)) ([902fc14](902fc14)) * **hugr-py:** docstrings for builder ([#1231](#1231)) ([3e4ac18](3e4ac18)) ### Code Refactoring * Remove "Signature" from hugr-py ([#1186](#1186)) ([65718f7](65718f7)) * Remove NodeType and input_extensions ([#1183](#1183)) ([ea5213d](ea5213d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: hugrbot <[email protected]> Co-authored-by: Seyon Sivarajah <[email protected]>

ss2165 requested a review from a team as a code owner June 5, 2024 16:38

ss2165 requested review from aborgna-q and mark-koch and removed request for aborgna-q June 5, 2024 16:38

ss2165 changed the base branch from main to ss/node-kids June 5, 2024 16:39

ss2165 changed the title ~~feat: automatically add state order edges for inter-graph edges~~ feat(hugr-py): automatically add state order edges for inter-graph edges Jun 6, 2024

mark-koch approved these changes Jun 6, 2024

View reviewed changes

ss2165 force-pushed the ss/auto-order branch 2 times, most recently from 273fe7e to a7ad654 Compare June 6, 2024 13:30

ss2165 added 3 commits June 6, 2024 14:31

feat: ancestral sibling function

a4b4487

fix: use -1 indexing to add state order edges

db5a504

feat: automatically add state order edges for inter-graph edges

e36df74

ss2165 force-pushed the ss/auto-order branch from a7ad654 to c899418 Compare June 6, 2024 13:32

ss2165 changed the base branch from ss/node-kids to main June 6, 2024 13:33

ss2165 enabled auto-merge June 6, 2024 13:33

ss2165 changed the base branch from main to ss/node-kids June 6, 2024 13:40

ss2165 changed the base branch from ss/node-kids to main June 6, 2024 13:40

feat: add order link iterators and fix detected bug

286e420

ss2165 force-pushed the ss/auto-order branch from c899418 to 286e420 Compare June 6, 2024 13:42

ss2165 added this pull request to the merge queue Jun 6, 2024

Merged via the queue into main with commit 5da06e1 Jun 6, 2024
20 checks passed

ss2165 deleted the ss/auto-order branch June 6, 2024 13:46

hugrbot mentioned this pull request Jun 6, 2024

chore(main): release hugr-py 0.3.0 #1092

Merged

ss2165 mentioned this pull request Jun 13, 2024

Automatically add state-order edges to inter-graph edges in python #1164

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(hugr-py): automatically add state order edges for inter-graph edges #1165

feat(hugr-py): automatically add state order edges for inter-graph edges #1165

ss2165 commented Jun 5, 2024

codecov bot commented Jun 5, 2024 •

edited

Loading

mark-koch left a comment

mark-koch Jun 6, 2024

ss2165 Jun 6, 2024

mark-koch Jun 6, 2024

ss2165 Jun 6, 2024

ss2165 commented Jun 6, 2024

	while (tgt_parent := h[tgt].parent) is not None:
	while tgt_parent := h[tgt].parent:

feat(hugr-py): automatically add state order edges for inter-graph edges #1165

feat(hugr-py): automatically add state order edges for inter-graph edges #1165

Conversation

ss2165 commented Jun 5, 2024

codecov bot commented Jun 5, 2024 • edited Loading

Codecov Report

mark-koch left a comment

Choose a reason for hiding this comment

mark-koch Jun 6, 2024

Choose a reason for hiding this comment

ss2165 Jun 6, 2024

Choose a reason for hiding this comment

mark-koch Jun 6, 2024

Choose a reason for hiding this comment

ss2165 Jun 6, 2024

Choose a reason for hiding this comment

ss2165 commented Jun 6, 2024

codecov bot commented Jun 5, 2024 •

edited

Loading