-
-
Notifications
You must be signed in to change notification settings - Fork 163
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
Reduce doc build time for learning representations #2165
Reduce doc build time for learning representations #2165
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2165 +/- ##
==========================================
+ Coverage 98.20% 98.26% +0.06%
==========================================
Files 88 88
Lines 4170 4146 -24
==========================================
- Hits 4095 4074 -21
+ Misses 75 72 -3 ☔ View full report in Codecov by Sentry. |
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.
Nice work! We love fast docs!
The one (nonblocking) question I have is if someone downloads this notebook (as a markdown file) and converts it a python notebook, will it run out of the box, or create an error looking for the PEC data file? Either way, I'm okay to merge. Just something I think we should be aware of/know the answer to.
LineQubit.range(2), n_moments=10, random_state=np.random.RandomState(1) | ||
LineQubit.range(2), n_moments=5, random_state=np.random.RandomState(1) |
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.
Why is it that this change does not require a change to the data that is loaded in further into the file? Naively, I would have thought if we are changing the circuit, then the PEC data would need to change as well. Does the PEC data consist of representations of the gates, which does not change here?
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.
The change is to match the circuits used to generate the file. The file was generated separately from this notebook and is already used in test_learning.py
to make the tests run efficiently.
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.
Ahh okay that makes sense! Thanks for the clarification. Can you comment on what's in the data file? (just for the sake of me learning!)
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.
An array of values of execute_with_pec
for different values of depolarizing noise strength epsilon
(equally spaced apart by ??). The zeroth column is the value of epsilon used in the representations for PEC.
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.
Makes sense! And the circuit used in the data is the the same as the one defined here?
mitiq/docs/source/examples/learning-depolarizing-noise.md
Lines 52 to 54 in a83f059
circuit = random_x_z_cnot_circuit( | |
LineQubit.range(2), n_moments=5, random_state=np.random.RandomState(1) | |
) |
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.
Yep!
If they "run all cells" it will error out if they don't have the data files, and it's a shame the "hidden" cell shows in the Jupyter notebook. But they can (and should) run the next cell, which does the real workflow with |
Description
Fixes #2075
Use cached PEC data and skip PEC executions in optimization loop, to speed up doc build time for
learning-depolarizing-noise.md
Local execution time: ~100 seconds --> ~4 seconds
Problem with optimization fixed (tested locally and on RTD)- was comparing ideal values of the wrong circuit. Now that the circuit matches the one used to generate the data, the optimizer converges to the correct value.
License
Before opening the PR, please ensure you have completed the following where appropriate.