resolve flaky mirror QV circuit test

[natestemen]

Description

Misty and I took a look at the recent failures we've been seeing with the test_generate_model_circuit_with_seed test which was originally raised in #2121. We weren't able to reproduce the failure locally, but we did find that the failure is due to tiny numerical differences in matrix gates coming from the inverse part of the mirror circuit. E.g.

(-0.41154946430369077+0.009252886470149477j),
                                        ^^^^^
(-0.41154946430369077+0.00925288647014949j), 
                                        ^^^^ 

While trying to fix this, we simplified some of the code to make it more readable (and probably a tiny bit faster). To fix the flaky test itself we use approximate equality instead of exact with an extremely small absolute tolerance to ensure the only differences are from numerics.

fixes #2121

Why can't we reproduce this locally?

The only ideas I have is that the chip running the computations to compute inverses of matrix gates is given more resources on my machine than the GitHub runner. If others have ideas, please say so!

History

Mirror Quantum Volume circuits were introduced in #1838, and indeed the first commit b9777e3 on the branch uses the simplest way to construct a mirror quantum volume circuit using something like the following.

qv_half = generate_quantum_volume_circuit(...)
mirror_qv = qv_half + cirq.inverse(qv_half)

A few commits later 2bf73b4, this code was substantially modified with commit message "mypy". Presumably this change was made to make mypy happy, but in doing so added a lot of complexity to the code. Even though this code works, it made debugging the code a bit harder than it would have otherwise. For that reason, and the general principle that readable code is (almost) always better, we've reverted to a simpler approach. With this approach we do have to use a typing.cast call to ensure mypy knows the correct type of the circuit returned from generate_quantum_volume_circuit.¹

Post-Mortem

This issue was made more difficult to debug due to the fact that code was rewritten to appease mypy². This, however, should not be the purpose of using mypy. We should do our best to help mypy understand the code we want to write, not rewrite code because mypy is complaining. At its best, mypy is a tool to help make our code better and more readable and at its worst, it can nudge us to rewrite clear code into something that is less readable despite passing mypy.

My recommendation here is to follow something like the following practice.

1. Write code as you wish!
2. Check types with mypy
3. If needed, help mypy understand by supplying more accurately typing. Try to avoid typing.cast and # type:ignore comments, but sometimes they are needed.

In particular, though, do not add code that will run (at runtime) only to appease mypy.

---

cc @purva-thakre since you wrote the original implementation, and @Misty-W since we paired on this together.

Footnotes

1. Which, BTW, is not known by mypy because the return type is a general QPROGRAM. ↩

2. My working theory from looking through the commit history. ↩

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0a9c202) 98.21% compared to head (28f02be) 98.21%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2127      +/-   ##
==========================================
- Coverage   98.21%   98.21%   -0.01%     
==========================================
  Files          87       87              
  Lines        4150     4145       -5     
==========================================
- Hits         4076     4071       -5     
  Misses         74       74              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Misty-W]

Thanks for the detailed summary @natestemen, LGTM!

[purva-thakre]

Thanks for catching this @natestemen!

I don't remember why I added L68-74, but commit history shows it's due to mypy? I need to look more into item 3 in the PR description.

Even though this code works, it made debugging the code a bit harder than it would have otherwise.

Apologies!

[natestemen]

No worries! Sometimes things get missed, or maybe there was actually a really good reason to have it that way that no longer is needed!