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 custom decompose method to gates.U3 and gates.CZ #1035

Merged
merged 5 commits into from
Oct 11, 2023

Conversation

renatomello
Copy link
Contributor

@renatomello renatomello commented Oct 10, 2023

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 Oct 10, 2023
@renatomello renatomello added this to the Qibo 0.2.2 milestone Oct 10, 2023
@renatomello renatomello self-assigned this Oct 10, 2023
@renatomello renatomello changed the title Add decompose method to gates.U3 Add custom decompose method to gates.U3 Oct 10, 2023
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

All modified lines are covered by tests ✅

Files Coverage Δ
src/qibo/gates/gates.py 100.00% <100.00%> (ø)

📢 Thoughts on this report? Let us know!.

Copy link
Member

@alecandido alecandido left a comment

Choose a reason for hiding this comment

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

Just a couple of comments about test and docs, but the feature on its own seems perfectly fine.

src/qibo/gates/gates.py Outdated Show resolved Hide resolved
tests/test_gates_gates.py Outdated Show resolved Hide resolved
@renatomello renatomello changed the title Add custom decompose method to gates.U3 Add custom decompose method to gates.U3 and gates.CZ Oct 10, 2023
@alecandido
Copy link
Member

@renatomello thanks to have applied the suggested corrections!

In general, it would be slightly better to pass a pRNG state, rather than using multiple seeds. It's because the properties of pRNG are optimized for sequential extractions, not necessarily for adjacent seeds.
However, this is certainly beyond the scope of this PR, since we should allow the various functions to get a state (i.e. a Generator object) instead of a seed.
And you should not notice any issue with just 10 extractions (these problems usually arise at the level of order of the generators' period extractions - or slightly less).

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 for adding this. Looks good to me.

Just noting that soon(ish) we will move the transpilers from qibolab to qibo and it would be great if these decompose methods are translated to transpilers to have a unified approach of gate transformations.

tests/test_gates_gates.py Show resolved Hide resolved
@renatomello
Copy link
Contributor Author

Thanks @renatomello for adding this. Looks good to me.

Just noting that soon(ish) we will move the transpilers from qibolab to qibo and it would be great if these decompose methods are translated to transpilers to have a unified approach of gate transformations.

Could you open an issue as a reminder for this?

@MatteoRobbiati MatteoRobbiati merged commit 1ecdf9a into master Oct 11, 2023
26 checks passed
@renatomello renatomello deleted the decompose_U3 branch October 11, 2023 09:45
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.

5 participants