[bugfix] Prevent GateOperation.add from superseding Circuit.radd

[daxfohl]

Per issue #7706, the change in #7651 caused Circuit.__radd__, which allows + to prepend operations to circuits, e.g. H(q) + Circuit(...) == Circuit(H(q), ...), to stop working. The root cause is that the change added a GateOperation.__add__, which thus supersedes Circuit.__radd__ and changes the behavior of + in such cases.

The fix for this is to modify the new GateOperation.__add__ (and GateOperation.__sub__, for symmetry) so that it only applies in cases where interpreting the sum as a PauliString is reasonable.

To do this, first, I updated GateOperation.__add__ with a check, if not isinstance(other, (Operation, Number)): return NotImplemented, since ops and numbers are the only things that can be interpreted as a PauliString. This change prevents GateOperation.__add__ from interfering with Circuit.__radd__, and constitutes the crux of the fix.

That change in isolation breaks GateOperation + PauliSum because the latter isn't an Operation. The simplest fix for this would be to add PauliSum to the new isinstance check, e.g. isinstance(other, (Operation, Number, PauliSum)), However that approach adds PauliSum as a dependency of GateOperation, which seems unnatural. So instead, I added logic in PauliSum.__add__ to convert ops to PauliString first. That approach doesn't create any new dependencies, and is also more robust because it works for all operations, not just GateOperations.

(Note, an even simpler fix for the entire issue would have been, instead of allowlisting (Operation, Number) in GateOperation.__add__, to denylist Circuit instead, e.g. if isinstance(other, Circuit): return NotImplemented. That would have fixed the issue and not required any special change for PauliSum. But similarly, I chose against that route because of the unnatural dependency it would create, as well as the fact that that fix wouldn't help if any third-party classes used radd on operations in the same way Circuit does.)

Finally, I updated the Circuit.radd unit test to include assertions that would have broken under the old code. The existing test used X gates everywhere, which didn't fail when prepending because of misc internal logic for Pauli gates. So I parameterized the test on gate, and added a non-Pauli gate H to the parameters, which will now fail if a similar regression is made in the future. I updated some tests in linear_combinations to test __sub__ more thoroughly as well, for symmetry.

Key takeaway: be careful when implementing __add__ for an existing class, and limit the scope of other to which it can apply, as otherwise the change can break __radd__ functionality in unrelated classes.

Fixes #7706

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.38%. Comparing base (57f17ce) to head (2891a77).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7707   +/-   ##
=======================================
  Coverage   99.38%   99.38%           
=======================================
  Files        1090     1090           
  Lines       98232    98248   +16     
=======================================
+ Hits        97627    97643   +16     
  Misses        605      605           

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pavoljuhas]

LGTM with 2 very minor suggestions.

Thank you for fixing this!