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

Operator converts tuple to np.ndarray #4022

Merged
merged 4 commits into from Apr 21, 2023
Merged

Conversation

albi3ro
Copy link
Contributor

@albi3ro albi3ro commented Apr 21, 2023

Currently, Operator converts tuples to np.ndarray upon initialization:

>>> op = qml.BasisState([1,0], wires=(0,1))
>>> op.data
[array([1, 0])]

But it does not do the same thing with tuples. Somehow previously, operators like BasisState could accept tuples as their parameters, and at some point (we think in Unwrap), they got converted to the expected np.ndarray. Since this no longer happens in Unwrap, we now explicitly convert tuples to numpy arrays as well.

Lightning tests used tuples, and thus expected this behaviour to still exist. Instead of changing those tests, I think it makes to continue accept tuples in addition to just lists.

Note: I moved some things around in test_operation.py so the tests would be a little bit better organized.

@albi3ro albi3ro requested review from mlxd and a team April 21, 2023 14:43
Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Thanks for the quick turnaround @albi3ro
For non-trainable parametric gates like BasisState I think the old behaviour that this returns to makes sense.

@mlxd mlxd added the gpu label Apr 21, 2023
@albi3ro albi3ro added this to the Release v0.30 milestone Apr 21, 2023
Copy link
Contributor

@timmysilv timmysilv 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!!

@codecov
Copy link

codecov bot commented Apr 21, 2023

Codecov Report

Merging #4022 (03793c0) into master (b9071bc) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4022   +/-   ##
=======================================
  Coverage   99.58%   99.58%           
=======================================
  Files         351      351           
  Lines       30847    30847           
=======================================
  Hits        30718    30718           
  Misses        129      129           
Impacted Files Coverage Δ
pennylane/operation.py 97.28% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@albi3ro albi3ro enabled auto-merge (squash) April 21, 2023 15:26
@albi3ro albi3ro merged commit d3b1eb6 into master Apr 21, 2023
46 checks passed
@albi3ro albi3ro deleted the cast-tuple-operator-parameters branch April 21, 2023 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants