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 tests for exponential extrapolation #953

Merged
merged 9 commits into from
Jul 30, 2024

Conversation

natestemen
Copy link
Contributor

Context: The last PR (first, second) to add exponential extrapolation capabilities to catalyst.

Description of the Change: This PR ensures that pennylane.transforms.exponential_extrapolate works inside catalyst's mitigate_with_zne function.

Benefits: ZNE has increased functionality.

Possible Drawbacks: As part of the testing, I've removed 0.1 and 0.2 from the list of values being used to create different circuits. This is because exponential extrapolation is not necessarily as stable as polynomial fitting. For example, when extrapolating near constant values exponential extrapolation can struggle due to the fact that a linear fit is first performed to understand the "direction" (or sign) of the exponential. If the slope is positive, the data is flipped to fit something that looks like $A\mathrm{e}^{-Bx} + C$ where $B\in\mathbb{R}_{> 0}$.

An example of this happening can be seen here.

>>> exponential_extrapolate([1,2,3], [0.3894, 0.3894183, 0.38941])
-1.0000000000000059e-06
>>> richardson_extrapolate([1,2,3], [0.3894, 0.3894183, 0.38941])
0.3893551000000072

If we want to ensure to continue testing values 0.1 and 0.2 for polynomial extrapolation, we can create separate tests. Let me know what would be best.

Related GitHub Issues:

fixes #754

Copy link

codecov bot commented Jul 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.93%. Comparing base (4f2b9f1) to head (127116f).
Report is 224 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #953   +/-   ##
=======================================
  Coverage   97.93%   97.93%           
=======================================
  Files          73       73           
  Lines       10338    10338           
  Branches     1170     1170           
=======================================
  Hits        10125    10125           
  Misses        170      170           
  Partials       43       43           

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

@natestemen
Copy link
Contributor Author

just pinging @rmoyard / @dime10 / @josh146 in case y'all didn't see the discord message.

Copy link
Contributor

@cosenal cosenal left a comment

Choose a reason for hiding this comment

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

Lgtm

frontend/test/pytest/test_mitigation.py Show resolved Hide resolved
frontend/test/pytest/test_mitigation.py Outdated Show resolved Hide resolved
@rmoyard rmoyard self-requested a review July 25, 2024 13:12
Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

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

@natestemen could keep all the values but add a skip in the test directly for those values only for exponential? With the skip add a comment about exponential not being stable.

@rmoyard
Copy link
Contributor

rmoyard commented Jul 29, 2024

Closes #754

@natestemen natestemen requested a review from rmoyard July 29, 2024 18:28
Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

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

Thanks 💯

@natestemen
Copy link
Contributor Author

@rmoyard just added a snippet to the changelog, and added a missing docstring to satisfy CI. Feel free to take another look, or merge!

@rmoyard rmoyard merged commit 0055f9e into PennyLaneAI:main Jul 30, 2024
42 checks passed
@natestemen natestemen deleted the nts-exp-extrap branch July 30, 2024 19:04
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.

Add exponential fitting functionality to ZNE
3 participants