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

Add conversion of controlled unitary gates from Qiskit to TKET. #372

Merged
merged 6 commits into from
Aug 20, 2024

Conversation

gkpotter
Copy link
Contributor

@gkpotter gkpotter commented Aug 13, 2024

Description

Add support for converting controlled unitary gates from Qiskit to TKET.

Related issues

Please mention any github issues addressed by this PR.

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.

Notes

I have yet to update the changelog/API docs, in case more changes or refactoring are required.

@cqc-melf
Copy link
Collaborator

Hi @gkpotter
Thank you for the PR.
There is one issues with the format check, could you take care of this?
Besides that it is looking good.

@CalMacCQ
Copy link
Contributor

CalMacCQ commented Aug 13, 2024

I think its just a case of running the black formatter on the qiskit convert file

black qiskit_convert.py

@gkpotter
Copy link
Contributor Author

gkpotter commented Aug 13, 2024

Oops sorry about that. Just re-ran black on qiskit_convert.py!

@cqc-melf
Copy link
Collaborator

Looks good, I have run the checks locally and they all work. It would be nice to add the conversion in the other direction, too. If you don't have the time to do that, I will add an issue for that and try to do it in the near future.

If you don't want to add the other direction, please update the changelog and I am happy to approve this.

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.

See comment above

@gkpotter
Copy link
Contributor Author

I updated the changelog. Unfortunately, right now I do not have time to implement conversion in the other direction.

@CalMacCQ
Copy link
Contributor

I updated the changelog. Unfortunately, right now I do not have time to implement conversion in the other direction.

Thanks for the useful contribution @gkpotter :) This reminded me that the qiskit conversion code should be refactored.

@cqc-melf Are you fine with this being merged now?

@cqc-melf
Copy link
Collaborator

I have added an issue for that, #378

@cqc-melf cqc-melf self-requested a review August 20, 2024 08:22
@cqc-melf cqc-melf merged commit f790f9e into CQCL:main Aug 20, 2024
6 checks passed
@CalMacCQ
Copy link
Contributor

@gkpotter This should now be in the 0.56 release. Thanks again for adding this.

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