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

Support basic single qubit controlled gates #89

Merged
merged 10 commits into from
Aug 5, 2024
Merged

Conversation

glanzz
Copy link
Contributor

@glanzz glanzz commented Jul 1, 2024

Description

  • The basic single controlled gate sets are supported and used widely used.
  • The multicontrolled gates can be considered in future versions

Checklist

  • I have performed a self-review of my code.
  • I have commented hard-to-understand parts of my code.
  • I have made corresponding changes to the public API documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the changelog with any user-facing changes.

- The single controlled gate sets are supported but requires a slight modification
- The multicontrolled gates can be considered in future versions
@glanzz glanzz requested a review from cqc-melf as a code owner July 1, 2024 19:39
Copy link
Collaborator

@cqc-melf cqc-melf 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 updated the branch, the checks should now work. One small question left.

Happy to make a release after this is sorted.

}


_known_quil_gate_rev = {v: k for k, v in _known_quil_gate.items()}

# Gates with single control
_single_control_gates = ["CH", "CY"]
Copy link
Collaborator

@cqc-melf cqc-melf Jul 22, 2024

Choose a reason for hiding this comment

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

Why is CNOT and CPhase not in this list?

Copy link
Collaborator

Choose a reason for hiding this comment

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

And also CZ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

CNOT would not work with the logic below because there is no NOT gate, I think.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You could do X for Cnot?

Copy link
Collaborator

@cqc-alec cqc-alec Jul 23, 2024

Choose a reason for hiding this comment

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

But line 311 assumes that the name from character 1 onwards is a valid gate name. This does seem like fragile assumption, and I think it would be better to implement this as a dictionary (name of controlled gate --> name of gate).

@cqc-melf
Copy link
Collaborator

cqc-melf commented Aug 2, 2024

The checks are still showing a problem, are you looking into that? @glanzz

@glanzz
Copy link
Contributor Author

glanzz commented Aug 2, 2024

Checking

@cqc-melf cqc-melf merged commit 011083e into CQCL:main Aug 5, 2024
6 checks passed
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.

3 participants