feat!: Make linear types @inout by default; add @owned annotation #486

croyzor · 2024-09-12T09:22:34Z

Resolves #413 and #414.

BREAKING CHANGE: Linear function arguments are now borrowed by default; removed the now redundant @inout annotation

codecov-commenter · 2024-09-12T09:25:46Z

Codecov Report

Attention: Patch coverage is 93.93939% with 6 lines in your changes missing coverage. Please review.

Project coverage is 91.99%. Comparing base (00b18c1) to head (fdf10f2).

Files with missing lines	Patch %	Lines
guppylang/prelude/quantum_functional.py	90.00%	5 Missing ⚠️
guppylang/tys/parsing.py	92.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #486      +/-   ##
==========================================
- Coverage   92.01%   91.99%   -0.03%     
==========================================
  Files          58       59       +1     
  Lines        5914     5921       +7     
==========================================
+ Hits         5442     5447       +5     
- Misses        472      474       +2

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

croyzor · 2024-09-12T09:43:15Z

guppylang/prelude/quantum_functional.py

+def rz(q: qubit @ owned, angle: angle) -> qubit: ...
+
+
+@guppy.hugr_op(quantum, quantum_op("Rx"))


N.B. This is broken in main. Both rz and rx are annoted with quantum_op("Rz")

croyzor · 2024-09-12T09:44:32Z

guppylang/checker/func_checker.py

@@ -163,7 +163,9 @@ def check_signature(func_def: ast.FunctionDef, globals: Globals) -> FunctionType
    )


-def parse_docstring(func_ast: ast.FunctionDef) -> tuple[ast.FunctionDef, str | None]:
+def parse_function_with_docstring(


Drive-by: I updated the name of this function because I found it too confusing

croyzor · 2024-09-12T09:45:21Z

guppylang/tys/parsing.py

@@ -82,11 +82,11 @@ def _try_parse_defn(node: AstNode, globals: Globals) -> Definition | None:
    match node:
        case ast.Name(id=x):
            if x not in globals:
-                raise GuppyError("Unknown identifier", node)
+                raise GuppyError(f"Unknown identifier: {x}", node)


Drive-by: Printing the unknown identifiers here and on line 89

See #479. It also makes the printed id show the module name instead of the object repr.

croyzor · 2024-09-12T09:47:24Z

tests/error/linear_errors/copy_qubit.py

@@ -1,6 +1,7 @@
 import guppylang.prelude.quantum as quantum
 from guppylang.decorator import guppy
 from guppylang.module import GuppyModule
+from guppylang.prelude.builtins import owned


I moved this file from copy.py to copy_qubit.py because of some weird case where it was being imported by the guppy prelude instead of the python copy library. Not 100% sure what was going on there but I don't want to touch it with a 10ft pole.

mark-koch

Very nice! 🎉

I think you also need to update the autogenerated constructors for structs (see guppylang/definition/structs.py) to take ownership of linear arguments. Surprising that this wasn't caught by the tests :/

guppylang/tys/parsing.py

mark-koch · 2024-09-12T10:06:28Z

guppylang/prelude/builtins.py

@@ -74,7 +74,14 @@ def __rmatmul__(self, other: Any) -> Any:
        return other


-inout = _Inout()


We should also remove the _Inout class

guppylang/tys/parsing.py

mark-koch · 2024-09-12T10:14:10Z

guppylang/tys/parsing.py

@@ -252,8 +256,11 @@ def type_from_ast(
 ) -> Type:
    """Turns an AST expression into a Guppy type."""
    ty, flags = type_with_flags_from_ast(node, globals, param_var_mapping)
-    if flags != InputFlags.NoFlags:
-        raise GuppyError("`@` type annotations are not allowed in this position", node)
+    if flags != InputFlags.NoFlags and flags != InputFlags.Inout:


Why exclude InputFlags.Inout here?

If a linear variable has InputFlags.Inout, it's because we set it - it can't have been done via an annotation from the user

But if it were set, this would be a compiler bug, right? Maybe do an assert instead?

Updated to an assert

mark-koch · 2024-09-12T10:15:52Z

pyproject.toml

+disable_error_code = ['valid-type' # Otherwise mypy will complain about @owned annotations
+                     ]


I think we shouldn't disable this globally, only for files that actually contain Guppy code

Removed and readded locally to the definitions in quantum.py and quantum_functional.py

mark-koch · 2024-09-12T10:20:24Z

tests/error/comprehension_errors/used_twice2.err

-17:    def foo(qs: linst[qubit]) -> linst[qubit]:
+17:    def foo(qs: linst[qubit] @owned) -> linst[qubit]:
 18:        return [q for q in qs if bar(q)]
-                   ^
-GuppyError: Variable `q` with linear type `qubit` was already used (at 18:33)
+                                        ^
+GuppyTypeError: Variable `q` with linear type `qubit` is not used on all control-flow paths of the list comprehension


You need to annotate bar as owned as well to reproduce the old error

Good spot! Updated

mark-koch · 2024-09-12T10:28:01Z

tests/error/linear_errors/__init__.err

Can you delete this file? Same for __pycache__.err

mark-koch · 2024-09-12T10:34:10Z

tests/error/poly_errors/non_linear2.err

-           ^^^^^^
-GuppyTypeError: Cannot instantiate non-linear type variable `T` in type `forall T. (T -> T) -> None` with linear type `qubit`
+               ^
+GuppyTypeError: Expected argument of type `?T -> ?T`, got `qubit -> qubit`


Why did this error get worse?

I think it's failing comparing T with NoFlags against qubit with Inout before it can get to the linearity check. I'll add a better error message for these flag mismatches

Actually, this is clearer after I fixed a bug locally - will push now. The printing code was hiding @owned flags. Now we get ?T -> ?T vs qubit @owned -> qubit

BUT the case I was thinking of in my first comment also happens. I've just added poly_errors/non_linear3.py to demonstrate this. @inout flags are silent during printing because they're not user-facing, but they still mess up unification. I'll add an issue for this

croyzor · 2024-09-13T11:21:18Z

I think you also need to update the autogenerated constructors for structs (see guppylang/definition/structs.py) to take ownership of linear arguments. Surprising that this wasn't caught by the tests :/

I've added a test and the logic for this, but it seems like it was working by accident! Something's a bit fishy there...

qartik · 2024-09-13T11:26:08Z

@croyzor could you also resolve the conflicts that github is pointing out in tests/integration/test_array.py?

Some leftover skips from the hugr update, and from #343 Also adds some extra info on the remaining skips. --------- Co-authored-by: Agustin Borgna <[email protected]>

mark-koch · 2024-09-13T12:12:26Z

guppylang/prelude/quantum.py

@@ -29,12 +29,12 @@ def dirty_qubit() -> qubit: ...


 @guppy.hugr_op(quantum, quantum_op("Measure"))
-def measure_return(q: qubit @ owned) -> tuple[qubit, bool]: ...
+def measure_return(q: qubit @ owned) -> tuple[qubit, bool]: ...  # type: ignore[valid-type]


You could also add valid-type to the mypy: disable-error-code pragma at the start of the file.

Same in quantum_functional.py

guppylang/tys/ty.py

guppylang/checker/linearity_checker.py

guppylang/tys/parsing.py

Co-authored-by: Mark Koch <[email protected]>

@inout

🤖 I have created a release *beep* *boop* --- ## [0.12.0](v0.11.0...v0.12.0) (2024-09-18) ### ⚠ BREAKING CHANGES * Pytket circuits loaded via a `py` expression no longer take ownership of the passed qubits. * Lists and function tensors are no longer available by default. `guppylang.enable_experimental_features()` must be called before compilation to enable them. * The `GuppyModule` argument is now optional for all decorators and no longer the first positional argument. Removed the explicit module objects `builtins`, `quantum`, and `angle`. * `quantum_functional` is now its own Guppy module and no longer implicitly comes with `quantum`. * Linear function arguments are now borrowed by default; removed the now redundant `@inout` annotation ### Features * Add functions to quantum module and make quantum_functional independent ([#494](#494)) ([0b0b1af](0b0b1af)) * Hide lists and function tensors behind experimental flag ([#501](#501)) ([c867f48](c867f48)) * Make linear types [@inout](https://github.com/inout) by default; add [@owned](https://github.com/owned) annotation ([#486](#486)) ([e900c96](e900c96)) * Only lower definitions to Hugr if they are used ([#496](#496)) ([cc2c8a4](cc2c8a4)) * Support implicit modules for all decorators and turn builtins into implicit module ([#476](#476)) ([cc8a424](cc8a424)) * Use inout for pytket circuits ([#500](#500)) ([a980ec2](a980ec2)) ### Bug Fixes * `angle` is now a struct and emitted as a rotation ([#485](#485)) ([992b138](992b138)) * Evade false positives for inout variable usage ([#493](#493)) ([6fdb5d6](6fdb5d6)) * Fix redefinition of structs ([#499](#499)) ([0b156e9](0b156e9)) * Initialise _checked in GuppyModule ([#491](#491)) ([3dd5dd3](3dd5dd3)), closes [#489](#489) * use correct array ops ([#503](#503)) ([720d8b8](720d8b8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

croyzor added 10 commits September 10, 2024 10:41

Drive-by: change name of parse_docstring

40e9701

Drive-by: Say which identifiers are unknown

ec3072e

checkpoint: integration tests

091802e

Update comprehension error tests

1edb8f0

Update inout error files

24e7d04

Update linear error files

d5db2e7

Update misc error tests

e64155f

Only print the missing module for attributes

8a0976d

Update more error files

b982bc4

Merge remote-tracking branch 'origin/main' into feat/owned

ea04238

croyzor mentioned this pull request Sep 12, 2024

Declare inout quantum operations #378

Closed

croyzor commented Sep 12, 2024

View reviewed changes

croyzor mentioned this pull request Sep 12, 2024

chore: Small error print improvements #479

Merged

mark-koch reviewed Sep 12, 2024

View reviewed changes

croyzor added 4 commits September 12, 2024 11:53

Fix printing of flags

217d471

Review comments

718c2c7

Add new test case

f07175a

Update error files

6fa60fd

croyzor mentioned this pull request Sep 12, 2024

Report errors where variables are instantiated as an @inout type #487

Closed

croyzor and others added 5 commits September 12, 2024 12:31

Review comment: update used_twice

8f9b7de

revert pyproject change

8c911ee

Shut up mypy

45be26d

Update references to @inout

11d9642

Merge branch 'main' into feat/owned

fe69a36

croyzor changed the title ~~feat!: Make linear types @inout by default; add @owned annotation without updating errors~~ feat!: Make linear types @inout by default; add @owned annotation Sep 12, 2024

croyzor marked this pull request as ready for review September 12, 2024 11:46

croyzor requested a review from a team as a code owner September 12, 2024 11:46

croyzor requested a review from qartik September 12, 2024 11:46

croyzor and others added 5 commits September 13, 2024 12:30

Take ownership of linear args to struct constructors

df989bb

Add tests for linear struct initialisation

fc75c81

Make check an assert

b8fe162

chore: Unskip now-passing tests (#475)

b13e547

Some leftover skips from the hugr update, and from #343 Also adds some extra info on the remaining skips. --------- Co-authored-by: Agustin Borgna <[email protected]>

Merge remote-tracking branch 'origin/main' into feat/owned

f32cd61

mark-koch approved these changes Sep 13, 2024

View reviewed changes

croyzor and others added 4 commits September 13, 2024 14:00

Update guppylang/tys/ty.py

a34d824

Co-authored-by: Mark Koch <[email protected]>

Update guppylang/checker/linearity_checker.py

46d32a1

Co-authored-by: Mark Koch <[email protected]>

Update guppylang/tys/parsing.py

7c7e2d0

Co-authored-by: Mark Koch <[email protected]>

Update tests; mypy annotations

fdf10f2

croyzor enabled auto-merge September 13, 2024 13:15

croyzor added this pull request to the merge queue Sep 13, 2024

Merged via the queue into main with commit e900c96 Sep 13, 2024
3 checks passed

croyzor deleted the feat/owned branch September 13, 2024 13:20

hugrbot mentioned this pull request Sep 13, 2024

chore(main): release 0.12.0 #488

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!: Make linear types @inout by default; add @owned annotation #486

feat!: Make linear types @inout by default; add @owned annotation #486

croyzor commented Sep 12, 2024 •

edited

Loading

codecov-commenter commented Sep 12, 2024 •

edited

Loading

croyzor Sep 12, 2024

croyzor Sep 12, 2024

croyzor Sep 12, 2024

aborgna-q Sep 12, 2024

croyzor Sep 12, 2024

mark-koch left a comment

mark-koch Sep 12, 2024

croyzor Sep 13, 2024

mark-koch Sep 12, 2024

croyzor Sep 12, 2024

mark-koch Sep 12, 2024

croyzor Sep 13, 2024

mark-koch Sep 12, 2024

croyzor Sep 13, 2024

mark-koch Sep 12, 2024

croyzor Sep 13, 2024

mark-koch Sep 12, 2024

croyzor Sep 13, 2024

mark-koch Sep 12, 2024

croyzor Sep 12, 2024

croyzor Sep 12, 2024 •

edited

Loading

croyzor Sep 12, 2024

croyzor Sep 12, 2024

croyzor commented Sep 13, 2024

qartik commented Sep 13, 2024

mark-koch Sep 13, 2024

		def rz(q: qubit @ owned, angle: angle) -> qubit: ...


		@guppy.hugr_op(quantum, quantum_op("Rx"))

		@@ -74,7 +74,14 @@ def __rmatmul__(self, other: Any) -> Any:
		return other


		inout = _Inout()

		disable_error_code = ['valid-type' # Otherwise mypy will complain about @owned annotations
		]

feat!: Make linear types @inout by default; add @owned annotation #486

feat!: Make linear types @inout by default; add @owned annotation #486

Conversation

croyzor commented Sep 12, 2024 • edited Loading

codecov-commenter commented Sep 12, 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

mark-koch 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

croyzor Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

croyzor commented Sep 13, 2024

qartik commented Sep 13, 2024

Choose a reason for hiding this comment

croyzor commented Sep 12, 2024 •

edited

Loading

codecov-commenter commented Sep 12, 2024 •

edited

Loading

croyzor Sep 12, 2024 •

edited

Loading