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

Refactor GeneralizedRotation to generators with (possible) nullspace. #306

Merged
merged 11 commits into from
Sep 22, 2023

Conversation

dariavh
Copy link
Contributor

@dariavh dariavh commented Sep 20, 2023

Hi Jakob,

Following our discussion about the GeneralizedRotation gate, I have transferred the shifted_gates functionality from QubitExcitation to GeneralizedRotation. This modification allows the GeneralizedRotation method to accept generators that do not necessarily have only two eigenvalues of +-r.

@dariavh dariavh changed the title Refactor GeneralizedRotation to generators with nullspace (P0). Refactor GeneralizedRotation to generators with (possible) nullspace. Sep 20, 2023
@@ -1056,14 +1052,13 @@ def __init__(self, angle, target, generator=None, p0=None, assume_real=True, con
assert generator is not None
assert p0 is not None

super().__init__(name="QubitExcitation", parameter=angle, target=target, control=control)
self.generator = generator
if control is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This part needs to go to the parent class (GeneralizedRotationImpl) as this modification of p0,in case a controlled gate is initialized, needs to happen there as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks for the comment!

@kottmanj
Copy link
Collaborator

It looks good so far. I think I found the reasons for the failed tests (see comments).
I'll stop annotating since you are currently modifying faster than I comment :D

@dariavh
Copy link
Contributor Author

dariavh commented Sep 21, 2023

@kottmanj Additional remark: The map_qubits method in the QubitExcitationImpl class has duplicate code from its base class QGateImpl. I can refactor the method to call the base class and additionally map the qubits in the p0 generator. Is this change acceptable to you?

@dariavh dariavh marked this pull request as ready for review September 21, 2023 11:56
@kottmanj
Copy link
Collaborator

Looks ready to go!
Thanks a lot.

@kottmanj kottmanj merged commit 0621616 into tequilahub:devel Sep 22, 2023
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