Skip to content

Fix test failure on differing precision of float in proto string#5911

Merged
CirqBot merged 2 commits intoquantumlib:masterfrom
pavoljuhas:fix-test-failure-on-float-representation
Oct 6, 2022
Merged

Fix test failure on differing precision of float in proto string#5911
CirqBot merged 2 commits intoquantumlib:masterfrom
pavoljuhas:fix-test-failure-on-float-representation

Conversation

@pavoljuhas
Copy link
Copy Markdown
Collaborator

@pavoljuhas pavoljuhas commented Oct 5, 2022

Problem: CI Pytest MacOS (3.10) fails because float value is output with
a lower precision than in the expected textproto,
https://github.com/quantumlib/Cirq/actions/runs/3192951160/jobs/5211002538.

Solution: Test with parameters that have exact decimal representation,
theta = 0.75, fatol = xatol = 2**-7 = 0.0078125.

Problem: CI Pytest MacOS (3.10) fails because float value is output with
a lower precision than in expected textproto.
(https://github.com/quantumlib/Cirq/actions/runs/3192951160/jobs/5211002538)

Solution: Update expected textproto to a lower precision and
replace high-precision values in the tested string.
@pavoljuhas pavoljuhas requested review from a team, cduck, verult, vtomole and wcourtney as code owners October 5, 2022 22:59
@CirqBot CirqBot added the Size: XS <10 lines changed label Oct 5, 2022
layer_str = str(new_layer)
# Fix precision issues
layer_str = re.sub(r'0.004999\d+', '0.005', layer_str)
layer_str = re.sub(r'\b0.78539818\d*', '0.7853982', layer_str)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we fix this by setting theta on the fsim gate to a different value that can be exactly represented by floats, e.g. theta=0.5? We could do something similar on fatol to avoid the need for the previous string substitution hack, setting it to something like 2**-7 == 0.0078125 instead of 0.005.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sure, that is nicer than substitution. Done in 6148c40.

@CirqBot CirqBot added the size: S 10< lines changed <50 label Oct 6, 2022
@pavoljuhas pavoljuhas added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Oct 6, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Oct 6, 2022
@CirqBot CirqBot merged commit c875124 into quantumlib:master Oct 6, 2022
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Oct 6, 2022
@pavoljuhas pavoljuhas deleted the fix-test-failure-on-float-representation branch October 6, 2022 19:55
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
…ntumlib#5911)

Problem: CI Pytest MacOS (3.10) fails because float value is output with
a lower precision than in the expected textproto,
https://github.com/quantumlib/Cirq/actions/runs/3192951160/jobs/5211002538.

Solution: Test with parameters that have exact decimal representation,
theta = 0.75,  fatol = xatol = 2**-7 = 0.0078125.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: S 10< lines changed <50 Size: XS <10 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants