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

Removed in-place inversion #122

Merged
merged 3 commits into from
Feb 10, 2023
Merged

Removed in-place inversion #122

merged 3 commits into from
Feb 10, 2023

Conversation

mudit2812
Copy link
Contributor

Replaced all instances of in-place inversion with qml.adjoint()

Copy link
Contributor

@AlbertMitjans AlbertMitjans left a comment

Choose a reason for hiding this comment

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

Amazing! Just one small suggestion, but it is not blocking. 💯 🚀

(g.H(0).dagger(), qml.Hadamard(0).inv()),
(g.H(0).dagger().dagger(), qml.Hadamard(0).inv().inv()),
(g.H(0).dagger(), qml.adjoint(qml.Hadamard(0))),
(g.H(0).dagger().dagger(), qml.Hadamard(0)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to be consistent with the changes

Suggested change
(g.H(0).dagger().dagger(), qml.Hadamard(0)),
(g.H(0).dagger().dagger(), qml.adjoint(qml.adjoint(qml.Hadamard(0)))),

Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing for all operations

Copy link
Contributor Author

@mudit2812 mudit2812 Feb 10, 2023

Choose a reason for hiding this comment

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

I did that before and the tests failed. If a pyquil gate is inverted multiple times, the code in pennylane_rigetti/converter.py:_is_inverted only checks whether a gate is inverted or not by counting the number of times a gate has been inverted and checking if the count is odd or even, so the cases where the pyquil gates have .dagger().dagger() should never have had .inv().inv() in the first place (which is the same as not doing anything anyway)

@mudit2812 mudit2812 merged commit f6f8dad into master Feb 10, 2023
@mudit2812 mudit2812 deleted the rm-inverse branch February 10, 2023 18:38
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.

2 participants