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

Create gates.CY #1103

Merged
merged 17 commits into from
Dec 1, 2023
Merged

Create gates.CY #1103

merged 17 commits into from
Dec 1, 2023

Conversation

renatomello
Copy link
Contributor

@renatomello renatomello commented Nov 22, 2023

This closes #1099 and needs qiboteam/qibojit#158 for numba tests to pass

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.

@renatomello renatomello added documentation Improvements or additions to documentation enhancement New feature or request labels Nov 22, 2023
@renatomello renatomello added this to the Qibo 0.2.3 milestone Nov 22, 2023
@renatomello renatomello self-assigned this Nov 22, 2023
Copy link
Member

@stavros11 stavros11 left a comment

Choose a reason for hiding this comment

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

Thanks @renatomello. Looks good to me, there is only an issue with tests. Using this and the corresponding branch for qibojit, the following test fails for me

tests/test_gates_density_matrix.py::test_controlled_by_one_qubit_gates[qibojit-numba-Y]

I have not checked what is wrong though. This is on CPU, I did not run on GPU yet.

Copy link
Contributor

@Simone-Bordoni Simone-Bordoni left a comment

Choose a reason for hiding this comment

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

Also for me it is ok, just fix the issue with the tests

@renatomello
Copy link
Contributor Author

@stavros11 @Simone-Bordoni I pushed the fix to qiboteam/qibojit#158 and one can test locally that all tests are passing

Copy link
Member

@stavros11 stavros11 left a comment

Choose a reason for hiding this comment

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

Thanks @renatomello, it works for me now. For the CI, you probably need to run poetry lock because you changed the pyproject, however I don't think this is needed as we will revert back to qibojit-main before merging.

I don't remember what we usually do in this situation, but most likely we need to release qibo 0.2.3, bump the dependency on qibojit's pyproject and run poetry lock there.

@renatomello
Copy link
Contributor Author

Thanks @renatomello, it works for me now. For the CI, you probably need to run poetry lock because you changed the pyproject, however I don't think this is needed as we will revert back to qibojit-main before merging.

I don't remember what we usually do in this situation, but most likely we need to release qibo 0.2.3, bump the dependency on qibojit's pyproject and run poetry lock there.

I reverted back the change

Co-authored-by: BrunoLiegiBastonLiegi <[email protected]>
@scarrazza
Copy link
Member

@renatomello does the windows failure is expected (due to the missing qibojit release)?

@renatomello
Copy link
Contributor Author

@renatomello does the windows failure is expected (due to the missing qibojit release)?

Yes

@scarrazza
Copy link
Member

Thanks, I will merge this PR as the last one before release.

@scarrazza scarrazza merged commit f83fc56 into master Dec 1, 2023
10 of 19 checks passed
@renatomello renatomello deleted the controlled_y branch December 2, 2023 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CY gate
5 participants