Skip to content

Conversation

pavoljuhas
Copy link
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 wcourtney, a team, vtomole, cduck and verult as code owners October 5, 2022 22:59
@CirqBot CirqBot added the Size: XS <10 lines changed label Oct 5, 2022
@@ -269,6 +269,7 @@ def test_xeb_to_calibration_layer():
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
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
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