Skip to content

Rerun flaky test in vpe_circuits_test.py#1339

Merged
mhucka merged 6 commits into
quantumlib:mainfrom
arettig:vpe_flakytest
Jun 17, 2026
Merged

Rerun flaky test in vpe_circuits_test.py#1339
mhucka merged 6 commits into
quantumlib:mainfrom
arettig:vpe_flakytest

Conversation

@arettig

@arettig arettig commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

This test was causing intermittent failures of the CI in the 5% of runs where the measured value is outside 2 standard deviations from the expected value. The built in @cirq.testing.retry_once_with_later_random_values decorator will now rerun the test with a new random seed.

The built in @cirq.testing.retry_once_with_later_random_values decorator
will rerun the test with a new random seed.
@arettig arettig requested a review from mhucka June 4, 2026 15:38

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request adds the @cirq.testing.retry_once_with_later_random_values decorator to the test_single_timestep test in src/openfermion/circuits/vpe_circuits_test.py to mitigate potential test flakiness due to random values. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@arettig

arettig commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Looks like @cirq.testing.retry_once_with_later_random_values was added in cirq v1.5.0 but our ci.yaml uses v1.4.1:

cirq-version: [ 1.4.1 ]

@mhucka Is continuing support for 1.4.1 a hard requirement, or is it ok to bump that up in the CI?

@mhucka

mhucka commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

@mhucka Is continuing support for 1.4.1 a hard requirement, or is it ok to bump that up in the CI?

It is probably not a hard requirement, and now that you mention it, we should upgrade it to support the latest Cirq version if possible.

The pin in dev_tools/requirements/deps/runtime is actually a bit wider:

cirq-core>=1.4.1,<1.6.0

It ends up being 1.4.1 when pip-compile processes all the transitive dependencies, but I can no longer remember what the problem was the last time I tried to go higher than 1.6.

I'll open an issue about this.

@mhucka

mhucka commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

@arettig Quick update: I believe I worked out the changes needed to the requirements to make it possible to get higher versions of Cirq on Python versions > 3.10. I should have a PR ready tomorrow.

@mhucka mhucka added the area/tests Involves testing and test cases label Jun 10, 2026
@arettig

arettig commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@mhucka Thanks for fixing the Cirq versioning! It looks like this flaky test fix works now.

@mhucka

mhucka commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

@mhucka Thanks for fixing the Cirq versioning! It looks like this flaky test fix works now.

There is one thing that needs to be adjusted in the current PR. The decorator cirq.testing.retry_once_with_later_random_values is only available in Cirq versions 1.5.0+, which only supports Python 3.11+, but OpenFermion still can be installed in Python 3.10. If someone tries to run the tests in Python 3.10, they'll get an error about the undefined symbol. So, we need to protect against that. One way is to put something like this at the top of the file:

# Try to import the retry decorator (introduced in Cirq 1.5.0).
# If running on Cirq < 1.5.0, fall back to a no-op decorator.
try:
    retry_once_with_later_random_values = cirq.testing.retry_once_with_later_random_values
except AttributeError:
    def retry_once_with_later_random_values(func):
        return func

and then change the use of the decorator to be

@retry_once_with_later_random_values
def test_single_timestep():

This is not a great solution, but it should work.

Can I leave it to you to make the necessary changes to this PR?

The flaky decorator was added to Cirq in version 1.5.0, so the
updated flaky test would fail if a previous version of Cirq was used.
A wrapper for the decorator was added to wrapped.py, which will now
define it manually if an old Cirq is used.
@arettig

arettig commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

I saw there is a wrapped.py file for sorting out versioning issues with Cirq so I moved this fix there. In the case of an older Cirq version, I manually define it using the implementation copied from Cirq rather than a dummy function.

@mhucka mhucka left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Whoa, even better. Thanks!

@mhucka mhucka enabled auto-merge June 16, 2026 23:50
@mhucka mhucka added this pull request to the merge queue Jun 17, 2026
Merged via the queue into quantumlib:main with commit b86d7c2 Jun 17, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/tests Involves testing and test cases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants