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.

Sign up for GitHub

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

Jump to bottom

[CQT-127] Allow Qubit to accept QubitLike #325

Merged
merged 34 commits into from
Oct 8, 2024
Merged

[CQT-127] Allow Qubit to accept QubitLike #325

merged 34 commits into from
Oct 8, 2024

Conversation

juanboschero
Copy link
Collaborator

  • Make QubitLike type

  • Reformat Qubit dataclass

juanboschero and others added 5 commits September 9, 2024 17:07
@juanboschero
[CQT 126] Verify dataclasses using __post_init__ (#319) (#321)
b67dfe9 
* Verify dataclasses using __post_init__

* Restructure tests and remove try-exceptions

* Fix typos

---------

Co-authored-by: Juan Boschero <juan.boschero@tno.nl>
[CQT-127] Allow Qubit to accept QubitLike
4ab5e1d
Accept QubitLike throughout opensquirrel
820d681
@juanboschero
Merge branch 'develop' into CQT-127-qubit-like-class
f167dc5
Happify ruff
0b31beb
rturrado
rturrado reviewed Oct 1, 2024
View reviewed changes
opensquirrel/ir.py Outdated Show resolved Hide resolved
rturrado
rturrado reviewed Oct 1, 2024
View reviewed changes
Copy link
Contributor

@rturrado rturrado left a comment

Choose a reason for hiding this comment

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

Beautiful work!

I've added a few comments and one check to review.

I wouldn't be sure to always stop using a Qubit(index) syntax. Sometimes it may be clearer to know the type of the arguments.

test/docs/test_tutorial.py Outdated Show resolved Hide resolved
rturrado
rturrado reviewed Oct 3, 2024
View reviewed changes
Copy link
Contributor

@rturrado rturrado left a comment

Choose a reason for hiding this comment

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

I have done a more in depth review now.

My main comment is:

  • I don't think we have to change to using QubitLike everywhere. For example, for internal functions, such as expand_ket, that we are alredy calling with operands of type Qubit.
  • It's enough letting the programer use integers instead of Qubit types in the code that is, let's say, exposed. For example, through the CircuitBuilder, when you construct a gate.

My other bigger concern was: can't we have an implicit "from int" constructor? But I see constructor overloading is not really a built-in Python feature.

Other than that, it looks great to me.

opensquirrel/ir.py Outdated Show resolved Hide resolved
opensquirrel/circuit_builder.py Show resolved Hide resolved
opensquirrel/exporter/quantify_scheduler_exporter.py Outdated Show resolved Hide resolved
opensquirrel/ir.py Outdated Show resolved Hide resolved
opensquirrel/ir.py Outdated Show resolved Hide resolved
opensquirrel/utils/matrix_expander.py Outdated Show resolved Hide resolved
test/test_ir.py Outdated Show resolved Hide resolved
rturrado
rturrado reviewed Oct 3, 2024
View reviewed changes
opensquirrel/ir.py Outdated Show resolved Hide resolved
rturrado
rturrado reviewed Oct 4, 2024
View reviewed changes
Copy link
Contributor

@rturrado rturrado left a comment

Choose a reason for hiding this comment

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

Some last comments.

Thanks for the changes.

opensquirrel/ir.py Outdated Show resolved Hide resolved
opensquirrel/ir.py Outdated Show resolved Hide resolved
opensquirrel/utils/matrix_expander.py Outdated Show resolved Hide resolved
rturrado
rturrado approved these changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@rturrado rturrado left a comment

Choose a reason for hiding this comment

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

I think all conversations are resolved now, so let's go!

@juanboschero juanboschero merged commit 4d76942 into develop Oct 8, 2024
14 checks passed
@juanboschero juanboschero deleted the CQT-127-qubit-like-class branch October 8, 2024 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rturrado rturrado rturrado approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@juanboschero @rturrado @S-Linde