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

Fix the derivative of MottonenStatePreparation where possible #5774

Merged
merged 11 commits into from
Jun 13, 2024

Conversation

dwierichs
Copy link
Contributor

@dwierichs dwierichs commented May 31, 2024

Context:
The decomposition of MottonenStatePreparation skips some gates for special parameter values/input states.
See the linked issue for details.

Description of the Change:
This PR introduces a check for differentiability so that the gates only are skipped when no derivatives are being computed.
Note that this does not fix the non-differentiability at other special parameter points that also is referenced in #5715 and that is being warned against in the docs already.
Also, the linked issue is about multiple operations and we here only address MottonenStatePreparation.

Benefits:
Fixes parts of #5715. Unblocks #5620 .

Possible Drawbacks:

Related GitHub Issues:
#5715

Copy link

codecov bot commented Jun 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.67%. Comparing base (45f8d4a) to head (48b3ef6).
Report is 251 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5774      +/-   ##
==========================================
- Coverage   99.68%   99.67%   -0.01%     
==========================================
  Files         420      420              
  Lines       40138    39846     -292     
==========================================
- Hits        40010    39716     -294     
- Misses        128      130       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@astralcai astralcai self-requested a review June 12, 2024 14:11
Copy link
Contributor

@astralcai astralcai 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 small suggestion regarding skipping test cases, otherwise LGTM!

Copy link
Contributor

@astralcai astralcai left a comment

Choose a reason for hiding this comment

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

LGTM! 🎸

Copy link
Contributor

@KetpuntoG KetpuntoG left a comment

Choose a reason for hiding this comment

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

LGTM! happy to approve once I cleared up a couple of doubts

First of all, I would like to make sure. The derivative of a function with real output (as an expected value), can it give a complex number?

Copy link
Contributor Author

@dwierichs dwierichs left a comment

Choose a reason for hiding this comment

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

I answered both the complex numbers and precision issues in the comment :)

Copy link
Contributor

@KetpuntoG KetpuntoG left a comment

Choose a reason for hiding this comment

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

Thanks for the clarifications! Ready to go 🚀

@dwierichs dwierichs added the merge-ready ✔️ All tests pass and the PR is ready to be merged. label Jun 12, 2024
@dwierichs dwierichs enabled auto-merge (squash) June 12, 2024 23:50
@dwierichs dwierichs merged commit eb5c192 into master Jun 13, 2024
40 checks passed
@dwierichs dwierichs deleted the mottonen-deriv branch June 13, 2024 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-ready ✔️ All tests pass and the PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants