Rerun flaky test in vpe_circuits_test.py#1339
Conversation
The built in @cirq.testing.retry_once_with_later_random_values decorator will rerun the test with a new random seed.
There was a problem hiding this comment.
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.
|
Looks like OpenFermion/.github/workflows/ci.yaml Line 203 in efaadae @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: 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. |
|
@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 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 # 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 funcand 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.
|
I saw there is a |
mhucka
left a comment
There was a problem hiding this comment.
Whoa, even better. Thanks!
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_valuesdecorator will now rerun the test with a new random seed.