feat: Make arrays iterable #632

mark-koch · 2024-11-07T12:05:22Z

Add ArrayIter type to the standard library
Add array.__iter__ method

Drive by: Fix array method compilers to handle generic array lengths

Note: The ArrayIter.__end__ method depends on a new array_map Hugr op that doesn't exist yet to discard linear arrays

Closes #617

mark-koch · 2024-11-07T12:06:36Z

guppylang/checker/expr_checker.py

+        result, result_ty = func.synthesize_call([node, *args], node, self.ctx)
+        return with_type(result_ty, result), result_ty


Drive-by: Annotate the AST node with the inferred type to avoid repeated checking

Does it make sense for synthesize_call to do this itself?

mark-koch · 2024-11-07T12:07:11Z

guppylang/decorator.py

+        # Return an actual Python `TypeVar` so it can be used as an actual type in code
+        # that is executed by interpreter before handing it to Guppy.
+        return TypeVar(name)


Otherwise, we couldn't write Generic[T, n]

codecov-commenter · 2024-11-07T12:09:04Z

Codecov Report

Attention: Patch coverage is 93.65079% with 4 lines in your changes missing coverage. Please review.

Project coverage is 92.48%. Comparing base (065e011) to head (2236ecf).

Files with missing lines	Patch %	Lines
guppylang/std/builtins.py	81.81%	4 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #632   +/-   ##
=======================================
  Coverage   92.48%   92.48%           
=======================================
  Files          66       66           
  Lines        7507     7549   +42     
=======================================
+ Hits         6943     6982   +39     
- Misses        564      567    +3

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

mark-koch · 2024-11-07T12:15:58Z

guppylang/prelude/_internal/compiler/array.py

+            build_expect_none(
+                cast(DfBase[ops.DfParentOp], func), func.inputs()[0], err_msg
+            )


Annoying that we have this cast here.

func is a Function which inherits DfBase[ops.FuncDefn] and ops.FuncDefn inherits ops.DfParentOp. The problem is that DfBase is not covariant, so this cast is technically not safe. But I also don't want to define build_expect_none only for DfBase[ops.FuncDefn]. Any ideas?

I presume you can't just make DfBase covariant...(that'd be the easy fix but still not the nice one).

How about build_expect_none[P](func: DfBase[P], ...) where P is a typing.TypeVar bounded by ops.DfParentOp, can you do that?

Sadly also doesn't work, mypy still complains :/

mark-koch · 2024-11-07T12:16:56Z

guppylang/tys/arg.py

-            case BoundConstVar(idx=idx):
+            case BoundConstVar(idx=idx, ty=NumericType(kind=NumericType.Kind.Nat)):


Drive-by: This case should only trigger for nat type args

acl-cqc

Looks good, a few suggestions, possibly some answers, and a few questions...

acl-cqc · 2024-11-12T09:06:30Z

guppylang/checker/expr_checker.py

+        result, result_ty = func.synthesize_call([node, *args], node, self.ctx)
+        return with_type(result_ty, result), result_ty


Does it make sense for synthesize_call to do this itself?

acl-cqc · 2024-11-12T09:13:10Z

guppylang/prelude/_internal/compiler/array.py

-    name: str, elem_ty: ht.Type, length: int, inp: list[ht.Type], out: list[ht.Type]
+    name: str,
+    elem_ty: ht.Type,
+    length: ht.TypeArg,


Presumably all these length: ints are changing to length: ht.TypeArg to support them being compile-time-nat "type" variables....

Exactly 👍

acl-cqc · 2024-11-12T09:16:11Z

guppylang/prelude/_internal/compiler/array.py

        """The length for the array op that is being compiled."""
        match self.type_args:
-            case [_, ConstArg(ConstValue(value=int(length)))]:
-                return length
+            case [_, ConstArg(const=const)]:


Suggested change

case [_, ConstArg(const=const)]:

case [_, ConstArg(const)]:

would presumably be equivalent. But (if my interpretation of the int --> ht.TypeArg change above is correct), do we also need to handle this not being a ConstArg but a variable?

ConstArg here means "compile-time constant", so including literals and variables. Terminology borrowed from rustc: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/consts/type.ConstKind.html

Might be worth a little comment # Inc. literals and variables - it's not super-obvious that a Const could include a variable!!

acl-cqc · 2024-11-12T09:21:26Z

guppylang/prelude/_internal/compiler/array.py

+            build_expect_none(
+                cast(DfBase[ops.DfParentOp], func), func.inputs()[0], err_msg
+            )


I presume you can't just make DfBase covariant...(that'd be the easy fix but still not the nice one).

How about build_expect_none[P](func: DfBase[P], ...) where P is a typing.TypeVar bounded by ops.DfParentOp, can you do that?

acl-cqc · 2024-11-12T09:22:38Z

guppylang/prelude/_internal/compiler/array.py

+    def compile(self, args: list[Wire]) -> list[Wire]:
+        # For linear array iterators, we need to make sure that all array elements have
+        # been taken out (i.e. replaced by `None`)
+        if self.elem_ty.type_bound() == ht.TypeBound.Any:


Isn't there something like self.elem_ty.is_linear() you can use here?

I don't think that exists: https://cqcl.github.io/hugr/generated/hugr.tys.Type.html

Oh this is not guppy-specific anymore, right

acl-cqc · 2024-11-12T09:26:20Z

guppylang/prelude/_internal/compiler/array.py

+            )
+            func.set_outputs(func.add_op(ops.Tag(0, none_ty)))
+            func = self.builder.load_function(func)
+            # Map it over the array so that the resulting array is no longer linear and


Ok sorry, you have got this comment - just put this at the top!! :)

acl-cqc · 2024-11-12T09:27:22Z

guppylang/prelude/_internal/compiler/prelude.py

+
+
+def build_expect_none(
+    builder: DfBase[ops.DfParentOp], result: Wire, error_msg: str, error_signal: int = 1


Suggest rename result -> input (or input_option if you like)

acl-cqc · 2024-11-12T09:28:28Z

guppylang/prelude/builtins.py

+    @guppy
+    @no_type_check
+    def __hasnext__(self: "ArrayIter[L, n]" @ owned) -> tuple[bool, "ArrayIter[L, n]"]:
+        return self.i < int(n), self


Yeah, that's pretty cool :). Is the op for typearg-to-runtime-value in this PR?

This was added in #618

acl-cqc · 2024-11-12T09:29:03Z

guppylang/prelude/builtins.py

+
+
+@guppy.custom(ArrayGetitemCompiler())
+def _array_unsafe_getitem(xs: array[L, n], idx: int) -> L: ...


Could this just be normal array_get followed by unwrap?

Not on guppy level since we don't have an option type yet

acl-cqc · 2024-11-12T09:31:24Z

tests/integration/test_array.py

+        for q in qs:
+            discard(q)
+
+    validate(module.compile())


I am just wondering here if you could check that the module does include the expect_none stuff, and then have a counterexample

qs = array(qubit(), qubit(), qubit()) [discard(q) for q in qs]

that does not need expect none (because the array returned by the comprehension is copyable)....I think??

(because the array returned by the comprehension is copyable)....I think??

Yes, indeed! Unfortunately we can't add this test since #613 isn't merged yet

acl-cqc

Sorry for dropping the ball on this one! Looks good to me, modulo I'm hoping #676 fixes that cast issue

🤖 I have created a release *beep* *boop* --- ## [0.14.0](v0.13.1...v0.14.0) (2024-12-19) ### ⚠ BREAKING CHANGES * Lists in `py(...)` expressions are now turned into Guppy arrays instead of lists. * `dirty_qubit` function removed * measure_return renamed to `project_z` ### Features * add `maybe_qubit` stdlib function ([#705](#705)) ([a49f70e](a49f70e)), closes [#627](#627) * add measure_array and discard_array quantum function ([#710](#710)) ([3ad49ff](3ad49ff)) * Add method to load pytket circuit without function stub ([#712](#712)) ([ee1e3de](ee1e3de)) * Add Option type to standard library ([#696](#696)) ([45ea6b7](45ea6b7)) * Allow generic nat args in statically sized ranges ([#706](#706)) ([f441bb8](f441bb8)), closes [#663](#663) * Array comprehension ([#613](#613)) ([fdc0526](fdc0526)), closes [#614](#614) [#616](#616) [#612](#612) * Implicit coercion of numeric types ([#702](#702)) ([df4745b](df4745b)), closes [#701](#701) * Load `pytket` circuit as a function definition ([#672](#672)) ([b21b7e1](b21b7e1)) * Make arrays iterable ([#632](#632)) ([07b9871](07b9871)) * qsystem std functions with updated primitives ([#679](#679)) ([b0f041f](b0f041f)) * remove dirty_qubit ([#698](#698)) ([78e366b](78e366b)) * Turn py expression lists into arrays ([#697](#697)) ([d52a00a](d52a00a)) * Unpacking assignment of iterable types with static size ([#688](#688)) ([602e243](602e243)) * update to hugr 0.10 and tket2 0.6 ([#725](#725)) ([63ea7a7](63ea7a7)) ### Bug Fixes * Accept non-negative int literals and py expressions as nats ([#708](#708)) ([a93d4fe](a93d4fe)), closes [#704](#704) * Allow borrowing inside comprehensions ([#723](#723)) ([02b6ab0](02b6ab0)), closes [#719](#719) * Detect unsupported default arguments ([#659](#659)) ([94ac7e3](94ac7e3)), closes [#658](#658) * docs build command ([#729](#729)) ([471b74c](471b74c)) * Ensure `int`s can be treated as booleans ([#709](#709)) ([6ef6d60](6ef6d60)), closes [#681](#681) * Fix array execution bugs ([#731](#731)) ([0f6ceaa](0f6ceaa)) * Fix implicit modules in IPython shells ([#662](#662)) ([4ecb5f2](4ecb5f2)), closes [#661](#661) * Properly report error for unsupported constants ([#724](#724)) ([d0c2da4](d0c2da4)), closes [#721](#721) * Properly report errors for unsupported expressions ([#692](#692)) ([7f24264](7f24264)), closes [#691](#691) * remove use of deprecated Ellipsis ([#699](#699)) ([b819a84](b819a84)) ### Documentation * Fix docs build ([#700](#700)) ([684f485](684f485)), closes [#680](#680) * fix README.md and quickstart.md ([#654](#654)) ([abb0221](abb0221)) --- 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: Seyon Sivarajah <[email protected]>

mark-koch added 4 commits November 4, 2024 16:47

feat: Generic function definitions

263d527

fix: Fix generic array functions

d1af305

Disable array results

a81e3f1

feat: Make arrays iterable

5fbad3e

mark-koch requested a review from a team as a code owner November 7, 2024 12:05

mark-koch requested review from acl-cqc and removed request for a team November 7, 2024 12:05

mark-koch commented Nov 7, 2024

View reviewed changes

mark-koch mentioned this pull request Nov 11, 2024

feat: Compile array comprehensions to Hugr #616

Merged

acl-cqc reviewed Nov 12, 2024

View reviewed changes

mark-koch added 5 commits November 14, 2024 09:38

Merge remote-tracking branch 'origin/main' into feat/generic-defs

64b425c

Update diagnostics

8c4f4d1

Add nat call and recurse tests

5efa146

Merge branch 'feat/generic-defs' into fix/generic-array

0270fca

Update diagnostics

0de51a3

Base automatically changed from fix/generic-array to main November 14, 2024 10:32

mark-koch added 6 commits November 14, 2024 17:10

Merge branch 'fix/generic-array' into feat/array-iter

73d37b7

Merge remote-tracking branch 'origin/main' into feat/array-iter

278bee5

Simplify match

8b4e2d8

Improve comment

76e3a00

Rename result to option

85488ac

Annotate type in synthesize_call

c2d0cec

mark-koch requested a review from acl-cqc November 14, 2024 17:35

acl-cqc approved these changes Dec 2, 2024

View reviewed changes

mark-koch and others added 2 commits December 2, 2024 11:20

Merge remote-tracking branch 'origin/main' into feat/array-iter

b24a90c

Generalize build_{expect_none,unwrap_{left,right}}, remove cast (#676)

4d43244

Add comment explaining const variables

2236ecf

mark-koch added this pull request to the merge queue Dec 2, 2024

Merged via the queue into main with commit 07b9871 Dec 2, 2024
3 checks passed

mark-koch deleted the feat/array-iter branch December 2, 2024 11:29

hugrbot mentioned this pull request Dec 2, 2024

chore(main): release 0.14.0 #655

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Make arrays iterable #632

feat: Make arrays iterable #632

mark-koch commented Nov 7, 2024

mark-koch Nov 7, 2024

acl-cqc Nov 12, 2024

mark-koch Nov 7, 2024

codecov-commenter commented Nov 7, 2024 •

edited

Loading

mark-koch Nov 7, 2024

acl-cqc Nov 12, 2024

mark-koch Nov 14, 2024

acl-cqc Dec 2, 2024

mark-koch Nov 7, 2024

acl-cqc left a comment

acl-cqc Nov 12, 2024

acl-cqc Nov 12, 2024

mark-koch Nov 14, 2024

acl-cqc Nov 12, 2024

mark-koch Nov 14, 2024

acl-cqc Dec 2, 2024

acl-cqc Nov 12, 2024

acl-cqc Nov 12, 2024

mark-koch Nov 14, 2024

acl-cqc Dec 2, 2024

acl-cqc Nov 12, 2024

acl-cqc Nov 12, 2024

acl-cqc Nov 12, 2024

mark-koch Nov 14, 2024

acl-cqc Nov 12, 2024

mark-koch Nov 14, 2024

acl-cqc Nov 12, 2024

mark-koch Nov 14, 2024

acl-cqc left a comment

		result, result_ty = func.synthesize_call([node, *args], node, self.ctx)
		return with_type(result_ty, result), result_ty

		case BoundConstVar(idx=idx):
		case BoundConstVar(idx=idx, ty=NumericType(kind=NumericType.Kind.Nat)):



		def build_expect_none(
		builder: DfBase[ops.DfParentOp], result: Wire, error_msg: str, error_signal: int = 1



		@guppy.custom(ArrayGetitemCompiler())
		def _array_unsafe_getitem(xs: array[L, n], idx: int) -> L: ...

feat: Make arrays iterable #632

feat: Make arrays iterable #632

Conversation

mark-koch commented Nov 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Nov 7, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acl-cqc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acl-cqc left a comment

Choose a reason for hiding this comment

codecov-commenter commented Nov 7, 2024 •

edited

Loading