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

change tolerance #519

Merged
merged 1 commit into from Feb 24, 2020
Merged

change tolerance #519

merged 1 commit into from Feb 24, 2020

Conversation

@mariaschuld
Copy link
Contributor

mariaschuld commented Feb 24, 2020

Context:

Fixes a bug a user found: AmplitudeEmbedding had a lower normalization tolerance than the qnode, leading to edge cases where almost normalized states were used in a simulation.

Description of the Change:

Update tolerance to qnode value of 1e-10.

Related GitHub Issues:

Discussion Forum thread https://discuss.pennylane.ai/t/help-with-custom-circuit-design-in-pennylane/343

@mariaschuld mariaschuld requested a review from antalszava Feb 24, 2020
@@ -220,7 +221,7 @@ def circuit(f=None):
else:
norm = np.sum(np.abs(features) ** 2)

if not np.isclose(norm, 1.0, atol=TOLERANCE, rtol=0):
if not np.isclose(norm, 1.0, atol=TOLERANCE):
if normalize or pad:

This comment has been minimized.

Copy link
@mariaschuld

mariaschuld Feb 24, 2020

Author Contributor

This second change is to have the condition exactly like in the qnode...

@mariaschuld mariaschuld self-assigned this Feb 24, 2020
@mariaschuld mariaschuld added the bug label Feb 24, 2020
@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 24, 2020

Codecov Report

Merging #519 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #519   +/-   ##
=======================================
  Coverage   98.91%   98.91%           
=======================================
  Files          74       74           
  Lines        4702     4702           
=======================================
  Hits         4651     4651           
  Misses         51       51
Impacted Files Coverage Δ
pennylane/templates/embeddings/amplitude.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a01c32e...f5ff411. Read the comment docs.

@mariaschuld mariaschuld merged commit 81ce08e into master Feb 24, 2020
5 checks passed
5 checks passed
sphinx
Details
black
Details
CodeFactor No issues found.
Details
Travis CI - Pull Request Build Passed
Details
codecov/project 98.91% (+0%) compared to 86609fd
Details
@mariaschuld mariaschuld deleted the fix_tolerance_amplitudeembedding branch Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.