-
Notifications
You must be signed in to change notification settings - Fork 1
Fix missing adjoint problem #562
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
Conversation
|
FYI, CI is currently broken on release (which causes the failure here), because QuEraComputing/kirin#536 (and maybe also QuEraComputing/kirin#539) have to be backported to kirin v0.17. |
|
@david-pl Ah gotcha! On another note I had to bump up the minimum version of kirin in the pyproject.toml because the Although release-0-7 seemed to be fine with 17.23 so I'm not quite sure what's going on here |
|
@johnzl-777 I'm not sure why you had to bump the minimal version to get CI to work. I mean, it's the right thing to do, but my guess is that semver took care of that before: setting the compat to |
It's because CI is setup such that it use uv.lock to install dependencies I believe, so it does not automatically upgrade to latest |
rafaelha
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, once CI passes.
|
CI is blocked by QuEraComputing/kirin#542 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
|
@kaihsin is this good to merge? |
This fixes two problems, one where Stim adjoint gates weren't appearing from SquinToStim when they should and another where the outputs of SquinU3ToClifford were not correct (because no
*_DAGgates were coming out)@rafaelha has already implemented a version of the fix on
mainready for the latest release while this should be compatible with the current stable release (:As a bit of a historical aside, there was some discussion to canonicalize adjoints (so it would be safer to assume the existence of only one or zero
Adjointsby the time somebody hits this rewrite) here #468 but that's not planned