feat: Hugr lowering for array indexing and add integration tests #422

mark-koch · 2024-08-28T11:26:04Z

Closes #418. See #415 for context.

Note that array.__getitem__ and array.__setitem__ are currently lowered to dummy Hugr ops. The "swapping with None" logic explained in #415 will follow in a future PR (see #419)

codecov-commenter · 2024-08-28T11:30:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.63%. Comparing base (ded9e1c) to head (0a8ff48).
Report is 1 commits behind head on feat/arrays.

Additional details and impacted files

@@               Coverage Diff               @@
##           feat/arrays     #422      +/-   ##
===============================================
+ Coverage        92.48%   92.63%   +0.14%     
===============================================
  Files               48       48              
  Lines             5256     5295      +39     
===============================================
+ Hits              4861     4905      +44     
+ Misses             395      390       -5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ss2165

Just some small nits and questions

ss2165 · 2024-08-28T14:42:05Z

guppylang/prelude/_internal/compiler.py

+            args=[arg.to_hugr() for arg in self.type_args],
+        )
+        node = self.builder.add_op(op, array, idx, elem)
+        return CallReturnWires(regular_returns=[], inout_returns=[node])


nit: clearer to be explicit

Suggested change

return CallReturnWires(regular_returns=[], inout_returns=[node])

return CallReturnWires(regular_returns=[], inout_returns=[node[0])

ss2165 · 2024-08-28T14:44:57Z

tests/integration/test_array.py

+
+    @guppy(module)
+    def main(ss: array[S, 10]) -> array[S, 10]:
+        # This will panic at runtime :(


what will the runtime issue be?

See the explanation in #415

ss2165 · 2024-08-28T14:46:55Z

tests/integration/test_array.py

+
+    @guppy(module)
+    def main(qs: array[qubit, 42] @inout, i: int) -> None:
+        foo(qs[i])


both this and the example above would fail if the argument to foo wasn't inout right?

ss2165 · 2024-08-28T14:48:37Z

guppylang/compiler/expr_compiler.py

+            if subscript.item.id not in self.dfg.locals:
+                self.dfg[subscript.item] = self.visit(subscript.item_expr)


just a side-thought but it is a bit confusing to read that you check for existence of item.idin dfg.locals but then just assign to dfg[item]. Does dfg need a__contains__ ?

@guppy

This is the feature branch for array indexing. Tracked by #253. The goal is to allow array indexing in `@inout` positions that ignores linearity constraints in the indices: ```python qs: array[qubit, 42] cx(qs[0], qs[1]) # Ok cx(qs[0], qs[0]) # Compiles, but will panic at runtime q = qs[0] # Error: Indexing only allowed in inout position ``` This is achieved by lowering `array[qubit]` to `array[qubit | None]` and making `array.__getitem__` and `array.__setitem__` swap in `None` whenever a qubit is taken out or put back in. The functions `array.__getitem__` and `array.__setitem__` take the array as `@inout`, so it suffices to apply the following desugaring: ```python cx(qs[expr1], qs[expr2]) # Desugars to idx1 = expr1 idx2 = expr2 tmp1 = qs.__getitem__(idx1) tmp2 = qs.__getitem__(idx2) cx(tmp1, tmp2) qs.__setitem__(tmp1, idx1) qs.__setitem__(tmp2, idx2) ``` This also means that arrays containing structs might behave slightly unexpectedly: ```python @guppy.struct class QubitPair: q1: qubit q2: qubit ps: array[QubitPair, 42] cx(ps[0].q1, ps[0].q2) # Panics at runtime :( # Since it desugars to idx1 = 0 idx2 = 0 tmp1 = ps.__getitem__(idx1) tmp2 = ps.__getitem__(idx2) # Panic: Struct at index 0 has already been replaced with None cx(tmp1.q1, tmp2.q2) ... ``` To solve this, we need to change the Hugr lowering of arrays to structually replace any occurrence of `qubit` with `qubit | None`, i.e. instead of doing `array[QubitPair | None]`, we would need to do `array[tuple[qubit | None, qubit | None]]`. I'll leave this to a future PR in case we are interested in that. The following feature PRs target this branch: * #420 * #421 * #422 * #447

🤖 I have created a release *beep* *boop* --- ## [0.10.0](v0.9.0...v0.10.0) (2024-09-11) ### ⚠ BREAKING CHANGES * Bumped the `hugr` dependency to `0.8.0` * `GuppyModule.load` no longer loads the content of modules but instead just brings the name of the module into scope. Use `GuppyModule.load_all` to get the old behaviour. * Removed `guppylang.hugr_builder.hugr.Hugr`, compiling a module returns a `hugr.Package` instead. ### Features * Add `__version__` field to guppylang ([#473](#473)) ([b996c62](b996c62)) * Add angle type ([#449](#449)) ([12e41e0](12e41e0)) * Add array literals ([#446](#446)) ([a255c02](a255c02)) * Add equality test for booleans ([#394](#394)) ([dd702ce](dd702ce)), closes [#363](#363) * Add pi constant ([#451](#451)) ([9d35a78](9d35a78)) * Add qualified imports and make them the default ([#443](#443)) ([553ec51](553ec51)) * Allow calling of methods ([#440](#440)) ([5a59da3](5a59da3)) * Allow imports of function definitions and aliased imports ([#432](#432)) ([e23b666](e23b666)) * Array indexing ([#415](#415)) ([2199b48](2199b48)), closes [#421](#421) [#422](#422) [#447](#447) * Inout arguments ([#311](#311)) ([060649b](060649b)), closes [#315](#315) [#316](#316) [#349](#349) [#344](#344) [#321](#321) [#331](#331) [#350](#350) [#340](#340) [#351](#351) * range() with single-argument ([#452](#452)) ([d05f369](d05f369)) * Skip checking of redefined functions ([#457](#457)) ([7f9ad32](7f9ad32)) * Support `nat`/`int` ↔ `bool` cast operations ([#459](#459)) ([3b778c3](3b778c3)) * Use `hugr-cli` for validation ([#455](#455)) ([1d0667b](1d0667b)) * Use cell name instead of file for notebook errors ([#382](#382)) ([d542601](d542601)) * Use the hugr builder ([536abf9](536abf9)) ### Bug Fixes * Fix and update demo notebook ([#376](#376)) ([23b2a15](23b2a15)) * Fix linearity checking bug ([#441](#441)) ([0b8ea21](0b8ea21)) * Fix struct definitions in notebooks ([#374](#374)) ([b009465](b009465)) ### Documentation * Update readme, `cargo build` instead of `--extra validation` ([#471](#471)) ([c2a4c86](c2a4c86)) ### Miscellaneous Chores * Update hugr to `0.8.0` ([#454](#454)) ([b02e0d0](b02e0d0)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

mark-koch added 2 commits August 28, 2024 12:16

feat: Type check array subscripts

db6698b

feat: Update linearity checker to handle subscripts

0ada32b

mark-koch requested a review from a team as a code owner August 28, 2024 11:26

mark-koch requested review from ss2165 and removed request for a team August 28, 2024 11:26

mark-koch mentioned this pull request Aug 28, 2024

feat: Array indexing #415

Merged

mark-koch force-pushed the array/compile branch from 39717c6 to 1755c5b Compare August 28, 2024 11:27

feat: Hugr lowering for array indexing and add integration tests

9c390ec

mark-koch force-pushed the array/compile branch from 1755c5b to 9c390ec Compare August 28, 2024 11:30

ss2165 approved these changes Aug 28, 2024

View reviewed changes

Base automatically changed from array/linearity to feat/arrays August 30, 2024 15:59

mark-koch added 2 commits August 30, 2024 17:03

Merge remote-tracking branch 'origin/feat/arrays' into array/compile

f6b4729

Review suggestions

0a8ff48

mark-koch merged commit 851c8d1 into feat/arrays Aug 30, 2024
2 checks passed

mark-koch deleted the array/compile branch August 30, 2024 16:09

mark-koch mentioned this pull request Sep 3, 2024

Hugr lowering for inout array indexing #418

Closed

hugrbot mentioned this pull request Sep 3, 2024

chore(main): release 0.10.0 #377

Merged

hugrbot mentioned this pull request Oct 21, 2024

chore(main): release 0.13.0 #518

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Hugr lowering for array indexing and add integration tests #422

feat: Hugr lowering for array indexing and add integration tests #422

mark-koch commented Aug 28, 2024

codecov-commenter commented Aug 28, 2024 •

edited

Loading

ss2165 left a comment

ss2165 Aug 28, 2024

ss2165 Aug 28, 2024

mark-koch Aug 30, 2024

ss2165 Aug 28, 2024

mark-koch Aug 30, 2024

ss2165 Aug 28, 2024

	return CallReturnWires(regular_returns=[], inout_returns=[node])
	return CallReturnWires(regular_returns=[], inout_returns=[node[0])

		if subscript.item.id not in self.dfg.locals:
		self.dfg[subscript.item] = self.visit(subscript.item_expr)

feat: Hugr lowering for array indexing and add integration tests #422

feat: Hugr lowering for array indexing and add integration tests #422

Conversation

mark-koch commented Aug 28, 2024

codecov-commenter commented Aug 28, 2024 • edited Loading

Codecov Report

ss2165 left a comment

Choose a reason for hiding this comment

ss2165 Aug 28, 2024

Choose a reason for hiding this comment

ss2165 Aug 28, 2024

Choose a reason for hiding this comment

mark-koch Aug 30, 2024

Choose a reason for hiding this comment

ss2165 Aug 28, 2024

Choose a reason for hiding this comment

mark-koch Aug 30, 2024

Choose a reason for hiding this comment

ss2165 Aug 28, 2024

Choose a reason for hiding this comment

codecov-commenter commented Aug 28, 2024 •

edited

Loading