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

Added optional params sub_bit_prec and epsilon to preprocess_lcu_coefficients_for_reversible_sampling #861

Merged
merged 6 commits into from
May 29, 2024

Conversation

Vicara12
Copy link
Contributor

@Vicara12 Vicara12 commented Apr 5, 2024

This PR solves issue #799. Now preprocess_lcu_coefficients_for_reversible_sampling accepts either a tolerance epsilon or a number of bits of precision sub_bit_prec, related as $\epsilon = 2^{-n_b}$.

@Vicara12
Copy link
Contributor Author

Vicara12 commented Apr 8, 2024

@tanujkhattar it seems that there is a problem with Pylint, if I run it locally it always gives errors for the same files (which have not been modified):

************* Module qualtran.surface_code.ui_test
qualtran/surface_code/ui_test.py:16:0: E0401: Unable to import 'dash.exceptions' (import-error)
************* Module qualtran.surface_code.ui
qualtran/surface_code/ui.py:19:0: E0401: Unable to import 'plotly.express' (import-error)
qualtran/surface_code/ui.py:20:0: E0401: Unable to import 'plotly.graph_objects' (import-error)
qualtran/surface_code/ui.py:21:0: E0401: Unable to import 'dash' (import-error)
qualtran/surface_code/ui.py:22:0: E0401: Unable to import 'dash.exceptions' (import-error)

Is this normal or I'm doing something wrong?

@fdmalone
Copy link
Collaborator

fdmalone commented May 6, 2024

Are you running ./check/pylint?

@fdmalone
Copy link
Collaborator

fdmalone commented May 6, 2024

You might need to reinstall the requirements,

pip install -r dev_tools/requirements/envs/dev.env.txt

@fdmalone fdmalone self-requested a review May 6, 2024 17:01
Copy link
Collaborator

@fdmalone fdmalone left a comment

Choose a reason for hiding this comment

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

Thanks! Just spell out the kwargs though.

@tanujkhattar
Copy link
Collaborator

@Vicara12 Can you please address the comments?

Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

I've addressed the comments and made some more minor improvements which will be useful in a followup PR I'm working on. We can merge this.

@tanujkhattar tanujkhattar enabled auto-merge (squash) May 29, 2024 20:54
@tanujkhattar tanujkhattar dismissed fdmalone’s stale review May 29, 2024 21:46

I've addressed all the comments so we can merge now.

@tanujkhattar tanujkhattar merged commit 2798cf4 into quantumlib:main May 29, 2024
7 checks passed
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.

None yet

3 participants