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

feat!: Allow linear data inside lists #531

Merged
merged 2 commits into from
Oct 3, 2024
Merged

feat!: Allow linear data inside lists #531

merged 2 commits into from
Oct 3, 2024

Conversation

mark-koch
Copy link
Collaborator

@mark-koch mark-koch commented Oct 2, 2024

Closes #530 and closes #524.

  • Changes the signature of list methods to allow linear list elements (implicitly using the inout feature).
  • Removes methods that are not supported for now (see Add missing list methods #528)
  • Small change to the unification logic to be a bit laxer about requiring matching @owned flags if one of the arguments is not linear (otherwise we couldn't unify L @owned with any classical type). We'll need to revisit that once we have flags that can be put on classical types.

BREAKING CHANGE: Unsupported list methods have been removed.

@mark-koch mark-koch requested a review from a team as a code owner October 2, 2024 12:29
@mark-koch mark-koch requested a review from ss2165 October 2, 2024 12:29
@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.57%. Comparing base (b4fae3f) to head (1488c05).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #531      +/-   ##
==========================================
- Coverage   91.63%   91.57%   -0.06%     
==========================================
  Files          59       59              
  Lines        6190     6161      -29     
==========================================
- Hits         5672     5642      -30     
- Misses        518      519       +1     

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

Comment on lines -6 to +7
^
GuppyTypeError: Expected argument of type `?T -> ?T`, got `qubit @owned -> qubit`
^^^^^^
GuppyTypeError: Cannot instantiate non-linear type variable `T` in type `forall T. (T -> T) -> None` with linear type `qubit`
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A side effect of the unification change is that we get a better error message here.

However, I think #487 still applies

Copy link
Member

@ss2165 ss2165 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, few queries on the unsupporteds


@guppy.hugr_op(unsupported_op("Length"))
def __len__(self: list[T]) -> int: ...
@guppy.hugr_op(unsupported_op("pop")) # TODO: check None and unwrap
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"pop" doesn't seem right here?


@guppy.hugr_op(unsupported_op("Pop"))
def __next__(self: list[T]) -> tuple[T, list[T]]: ...
@guppy.hugr_op(unsupported_op("length")) # TODO: inout return in wrong order
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this ordering being resolved elsewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually turns out that the length op doesn't return the list at all! CQCL/hugr#1543


@guppy.custom(checker=FailingChecker("Guppy lists are immutable"))
def remove(self: list[T], elt: T) -> None: ...
@guppy.hugr_op(unsupported_op("Pop"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case difference between "pop" and "Pop" ?

@mark-koch mark-koch requested a review from ss2165 October 2, 2024 13:58
@mark-koch mark-koch changed the title feat: Allow linear data inside lists feat!: Allow linear data inside lists Oct 3, 2024
@mark-koch mark-koch added this pull request to the merge queue Oct 3, 2024
Merged via the queue into main with commit 229be2e Oct 3, 2024
4 of 5 checks passed
@mark-koch mark-koch deleted the feat/linear-lists branch October 3, 2024 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow linear types inside lists Use inout for lists
3 participants