Extract Pauli flow from XZ-corrections (closes #432)#526
Conversation
Implement `XZCorrections.to_pauli_flow` and the convenience method `Pattern.extract_pauli_flow`, reconstructing a Pauli flow directly from a pattern's XZ-corrections (Theorem 4 of Browne et al. 2007) rather than from the underlying open graph (whose Pauli flow is not unique and need not generate the pattern). The difficulty is the anachronical corrections: corrections targeting X/Y Pauli-measured nodes in the present or past of the corrected node. These are dropped by `PauliFlow.to_corrections` (the `& future` filter) and so never appear in the pattern, so they must be reconstructed. For each measured node this is cast as a GF(2) linear system: the future membership of the correction set is pinned by the observed X-corrections; the free variables are the anachronical (non-future, X/Y-measured) candidates and, where allowed, the node itself; and the equations encode the odd-neighbourhood constraints (Z-corrections on future nodes, P2 on past non-(Y/Z) nodes, the P3 coupling on past Y nodes, and the local proposition P4-P9 on the node). The system is solved over GF(2) with `_solve_gf2`. Tests verify, on the three worked examples of the issue, on a Pauli-measured open graph, and on a randomized family of open graphs that admit a Pauli flow, that the reconstructed flow is well formed and that `to_corrections()` reproduces the pattern's corrections exactly (the decisive round-trip criterion). Passes ruff, mypy --strict, pyright, and pytest locally. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #526 +/- ##
==========================================
+ Coverage 88.85% 89.03% +0.18%
==========================================
Files 49 49
Lines 7135 7225 +90
==========================================
+ Hits 6340 6433 +93
+ Misses 795 792 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Cover the branches where no Pauli flow is compatible with the XZ-corrections: a measured input node that must correct itself, and an isolated XY-measured node whose proposition P4 cannot be satisfied (unsolvable GF(2) system). Addresses the patch-coverage gap reported on the PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thierry-martinez
left a comment
There was a problem hiding this comment.
Thank you very much for this clean solution. Here are my initial comments on your PR.
| raise PartialOrderLayerError(PartialOrderLayerErrorReason.FirstLayer, layer_index=0, layer=first_layer) | ||
|
|
||
|
|
||
| def _solve_gf2(matrix: list[list[int]], rhs: list[int], n_vars: int) -> list[int] | None: |
There was a problem hiding this comment.
There is already a GF(2)-linear-system solver implemented in _linalg.solve_f2_linear_system.
| lambda r: Measurement.XY(round(float(r.random()), 3)), | ||
| lambda r: Measurement.XZ(round(float(r.random()), 3)), | ||
| lambda r: Measurement.YZ(round(float(r.random()), 3)), |
There was a problem hiding this comment.
Why do you choose to apply round there rather than leave the random float unchanged?
| pattern = OpenGraph( | ||
| graph=graph, input_nodes=inputs, output_nodes=outputs, measurements=measurements | ||
| ).to_pattern() | ||
| except Exception: # noqa: BLE001, S112 open graph without a flow -> not a valid test case |
There was a problem hiding this comment.
What are the relevant exceptions here? We should at least catch OpenGraphError. Are there any others?
| """ | ||
| correction_function = _reconstruct_pauli_correction_function(self) | ||
| pf: PauliFlow[_AM_co] = PauliFlow(self.og, correction_function, self.partial_order_layers) | ||
| pf.check_well_formed() # Raises a `FlowError` if the reconstructed flow is not well formed. |
There was a problem hiding this comment.
Is it possible to fail at this point? Can you give an example where the correction_function isn’t well‑formed by construction?
- Reuse `graphix._linalg.solve_f2_linear_system` rather than the ad-hoc `_solve_gf2` (which is removed). The per-node augmented matrix `[A | b]` is reduced to row echelon form with `MatGF2.gauss_elimination(ncols=n_vars)`, inconsistency is detected by scanning for `[0...0 | 1]` rows, and the reduced system is then handed to the existing solver. - Update the `XZCorrections.to_pauli_flow` docstring to point at the existing GF(2) solver and to spell out the propositions encoded by the GF(2) system (P1 by construction via the X/Y-axis candidate restriction; P2-P9 directly). - Add a comment on the `pf.check_well_formed()` call: it is a regression guard; the algorithm satisfies the propositions by construction, so a failure there would indicate a bug rather than malformed input. - In the randomized round-trip test, narrow `except Exception` to `OpenGraphError` (the only documented raise of `OpenGraph.to_pattern` when no flow exists) and drop the cosmetic `round` on the random measurement angles. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the thoughtful review, @thierry-martinez! Pushed 9a73dc7 addressing each point:
Let me know if you’d like any other changes. |
|
Hello - you are currently over the PR limit for unitaryHACK, so your PRs will be disregarded until you align with the rules. Reach out to hack@unitary.foundation to be reinstated. |
matulni
left a comment
There was a problem hiding this comment.
Thanks for this nice work! Here are some comments from my first pass. There's some work before merging, but in the mean time I'm ok with assigning you the issue.
For my own curiosity, would you be willing to share the promtps and skills (or even the reasoning log) of the LLM you used to code this ? This is of course not mandatory, but I'm curious to see how the solution was found. Thanks!
| See :meth:`XZCorrections.to_pauli_flow`. | ||
| """ | ||
| og = xz.og | ||
| adjacency: dict[int, set[int]] = {n: set(og.graph.neighbors(n)) for n in og.graph.nodes} |
There was a problem hiding this comment.
| adjacency: dict[int, set[int]] = {n: set(og.graph.neighbors(n)) for n in og.graph.nodes} | |
| adjacency: dict[int, set[int]] = {n: og.neighbors({n}) for n in og.graph.nodes} |
| for i in range(lhs.shape[0]): | ||
| if not lhs[i].any() and b[i] != 0: | ||
| return None | ||
| solution = solve_f2_linear_system(lhs, MatGF2(b)) |
There was a problem hiding this comment.
| solution = solve_f2_linear_system(lhs, MatGF2(b)) | |
| # `solve_f2_linear_system` does not check if a solution exists or if `lhs` is in REF. | |
| solution = solve_f2_linear_system(lhs, MatGF2(b)) |
| solution = solve_f2_linear_system(lhs, MatGF2(b)) | ||
|
|
||
| correction_set = set(fixed_in_p) | ||
| correction_set.update(v for v, bit in zip(variables, solution, strict=True) if int(bit)) |
There was a problem hiding this comment.
| correction_set.update(v for v, bit in zip(variables, solution, strict=True) if int(bit)) | |
| correction_set.update(v for v, bit in zip(variables, solution, strict=True) if bit) |
| candidates = sorted(a for a in nonfuture_others if a in non_inputs and labels.get(a) in {Axis.X, Axis.Y}) | ||
| variables = [*candidates, node] if self_is_var else list(candidates) |
There was a problem hiding this comment.
I don't think sorting is necessary, but if it is, please add a test which would fail if candidates are not sorted.
| candidates = sorted(a for a in nonfuture_others if a in non_inputs and labels.get(a) in {Axis.X, Axis.Y}) | |
| variables = [*candidates, node] if self_is_var else list(candidates) | |
| variables = [a for a in nonfuture_others | {node} if a in non_inputs and labels.get(a) in {Axis.X, Axis.Y}] |
With the suggestion above, self_is_var is no longer necessary.
| # P2: the odd neighbourhood vanishes on non-future, non-(Y/Z) nodes. | ||
| for g in nonfuture_others: | ||
| lab_g = labels.get(g) | ||
| if lab_g is not None and lab_g not in {Axis.Y, Axis.Z}: | ||
| matrix.append(row_at(g)) | ||
| rhs.append(const_at(g)) | ||
|
|
||
| # P3: a non-future Y-measured node `g` must lie outside the closed odd neighbourhood of the | ||
| # correction set, i.e. its membership and odd-neighbourhood membership must coincide. | ||
| for g in nonfuture_others: | ||
| if labels.get(g) == Axis.Y: | ||
| row = row_at(g) | ||
| if g in var_index: | ||
| row[var_index[g]] ^= 1 | ||
| matrix.append(row) | ||
| rhs.append(const_at(g)) |
There was a problem hiding this comment.
I believe the following is equivalent and more clear:
| # P2: the odd neighbourhood vanishes on non-future, non-(Y/Z) nodes. | |
| for g in nonfuture_others: | |
| lab_g = labels.get(g) | |
| if lab_g is not None and lab_g not in {Axis.Y, Axis.Z}: | |
| matrix.append(row_at(g)) | |
| rhs.append(const_at(g)) | |
| # P3: a non-future Y-measured node `g` must lie outside the closed odd neighbourhood of the | |
| # correction set, i.e. its membership and odd-neighbourhood membership must coincide. | |
| for g in nonfuture_others: | |
| if labels.get(g) == Axis.Y: | |
| row = row_at(g) | |
| if g in var_index: | |
| row[var_index[g]] ^= 1 | |
| matrix.append(row) | |
| rhs.append(const_at(g)) | |
| for g in nonfuture_others: | |
| lab_g = labels.get(g) | |
| # `nonfuture_others` never contains output nodes | |
| assert lab_g is not None | |
| # P2: the odd neighbourhood vanishes on non-future, non-(Y/Z) nodes. | |
| if lab_g not in {Axis.Y, Axis.Z}: | |
| matrix.append(row_at(g)) | |
| rhs.append(const_at(g)) | |
| # P3: a non-future Y-measured node `g` must lie outside the closed odd neighbourhood of the | |
| # correction set, i.e. its membership and odd-neighbourhood membership must coincide. | |
| elif lab_g == Axis.Y: | |
| row = row_at(g) | |
| if g in var_index: | |
| row[var_index[g]] ^= 1 | |
| matrix.append(row) | |
| rhs.append(const_at(g)) |
| # No correction set reconciles the XZ-corrections with the Pauli-flow propositions. | ||
| raise FlowError |
There was a problem hiding this comment.
It may be worth adding a new FlowGenericErrorReason in flow.exceptions.py to pass your comment as an exception message.
| # Defensive: by construction the GF(2) equations of `_reconstruct_pauli_correction_function` | ||
| # encode propositions P2-P9 exactly, and the anachronical candidates are restricted to X/Y | ||
| # axes which guarantees P1; the general flow properties are also satisfied by construction | ||
| # (the correction function is defined on the measured nodes and its image is included in | ||
| # the non-input nodes). This check is kept as a regression guard. | ||
| pf.check_well_formed() |
There was a problem hiding this comment.
I agree with @thierry-martinez. This seems to be a purely defensive test, I'd be inclined to remove it and eventually add a more comprehensive test suite.
There was a problem hiding this comment.
Actually, the current implementation contains a case where this check can fail:Pattern().extract_xzcorrections().to_pauli_flow() raises graphix.flow.exceptions.PartialOrderError: The partial order cannot be empty.
However, this seems to be a bug in check_well_formed (see #531).
I agree with @matulni : these sanity checks should be part of the test-suite and should not be performed systematically in production.
| # succeeds) are converted to a pattern, and the reconstructed flow is checked to be | ||
| # well formed and to reproduce the pattern's corrections. | ||
| tested = 0 | ||
| for seed in range(400): |
There was a problem hiding this comment.
Having the seed as an internal parameter makes it difficult to debug. Could you please pass it as pytest.mark.parametrize variable ?
There was a problem hiding this comment.
In addition to these tests, could you please mirror the tests test_extract_causal_flow_rnd_circuit, test_extract_gflow_rnd_circuit, test_extract_causal_flow, and test_extract_gflow in test_pattern.py?
| assert tested >= 30 # ensure the randomized sweep actually exercised the extraction | ||
|
|
||
|
|
||
| def test_to_pauli_flow_raises_when_no_flow_exists() -> None: |
There was a problem hiding this comment.
Please use pytest.mark.parametrize. I'd appreciate if you could add a few more failure tests.
| _assert_round_trip(pattern) | ||
|
|
||
|
|
||
| @pytest.mark.filterwarnings("ignore:Open graph with non-inferred Pauli measurements.") |
There was a problem hiding this comment.
I believe that such warnings should not be triggered here; and if they are, we should address them, because whether Pauli measurements are inferred (or not) affects the Pauli flow.
| ] | ||
|
|
||
|
|
||
| @pytest.mark.filterwarnings("ignore:Open graph with non-inferred Pauli measurements.") |
There was a problem hiding this comment.
Same comment as above: these warnings should not be triggered, and they are relevant with respect to Pauli flows.
| # Defensive: by construction the GF(2) equations of `_reconstruct_pauli_correction_function` | ||
| # encode propositions P2-P9 exactly, and the anachronical candidates are restricted to X/Y | ||
| # axes which guarantees P1; the general flow properties are also satisfied by construction | ||
| # (the correction function is defined on the measured nodes and its image is included in | ||
| # the non-input nodes). This check is kept as a regression guard. | ||
| pf.check_well_formed() |
There was a problem hiding this comment.
Actually, the current implementation contains a case where this check can fail:Pattern().extract_xzcorrections().to_pauli_flow() raises graphix.flow.exceptions.PartialOrderError: The partial order cannot be empty.
However, this seems to be a bug in check_well_formed (see #531).
I agree with @matulni : these sanity checks should be part of the test-suite and should not be performed systematically in production.
…check Responds to @thierry-martinez's review on TeamGraphix#526: - Remove the two @pytest.mark.filterwarnings("ignore:Open graph with non-inferred Pauli measurements.") suppressions. The warning does not actually fire for these tests (verified by promoting it to an error), so the suppressions were unnecessary and masked a semantically relevant signal. - Stop running pf.check_well_formed() systematically in production inside XZCorrections.to_pauli_flow: the flow is well formed by construction, and the sanity check is exercised in the test-suite via is_well_formed(). This also avoids the production manifestation of the empty-partial-order bug (TeamGraphix#531): Pattern().extract_xzcorrections().to_pauli_flow() no longer raises PartialOrderError. - Add test_to_pauli_flow_empty_pattern covering that empty-pattern case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks, @thierry-martinez — both points addressed in the latest commit. Non-inferred Pauli warning suppressions. You're right that these shouldn't be silenced. I removed both
This also removes the production manifestation of #531: |
…trized tests - core.py: simplify the GF(2) variable selection (drop `self_is_var`/sorting), merge the P2/P3 constraint loops into one with an explanatory comment, add a general comment describing what `matrix`/`rhs` encode, and tidy the `adjacency`/`solve_f2_linear_system`/`correction_set.update` lines. - exceptions.py: add `FlowGenericErrorReason.NoPauliFlow`; raise the typed `FlowGenericError(NoPauliFlow)` instead of a bare `FlowError` when no Pauli flow reconciles the XZ-corrections. - tests: parametrize the randomized round-trip over the seed (one case per seed, skipping draws with no edges / no flow) and parametrize the no-flow failure cases (asserting the typed reason), adding XZ/YZ-input cases. - test_pattern.py: mirror the causal/gflow extraction tests for the Pauli flow (`test_extract_pauli_flow_rnd_circuit` and `test_extract_pauli_flow`). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the careful review, @matulni — all addressed in the latest push. Reconstruction (
Typed error. Added Defensive check. Removed Tests.
On the LLM question — I did use an AI coding assistant (Claude) and then verified everything myself by running the full test suite, which is how the edge cases (the anachronical corrections, the empty-pattern path) got shaken out. I'd rather keep my specific prompts and workflow to myself, but appreciate the curiosity — and I'm glad to keep iterating on the PR the normal way. |
Closes #432.
Implements
XZCorrections.to_pauli_flowand the convenience methodPattern.extract_pauli_flow(analogous toextract_causal_flow/extract_gflow), reconstructing a Pauli flow directly from the pattern's XZ-corrections (Theorem 4 of Browne et al. 2007) rather than from the underlying open graph — whose Pauli flow is not unique and is not guaranteed to generate the pattern.How the anachronical corrections were tackled (the crux of the issue)
The hard part, as the issue notes, is that a Pauli flow's correction sets may contain anachronical corrections: corrections targeting X/Y Pauli-measured nodes that lie in the present or past of the corrected node.
PauliFlow.to_correctionsdiscards these (thecorrecting_set & futurefilter), so they never appear in the pattern and cannot be read off the corrections — they must be reconstructed.For each measured node
i, reconstruction is cast as a linear system over GF(2):i, membership in the correction setp(i)is fixed by the observed X-corrections ofi(and must reproduce the Z-corrections via the odd neighbourhood). These become constants of the system.p(i)by P1) — and, where the local proposition allows it,iitself.i(P4–P9, handling the XY/XZ/YZ planes and the X/Y/Z axes).The system is solved with a small GF(2) Gaussian-elimination helper (
_solve_gf2); failure to solve at any node means no Pauli flow is compatible with the corrections. The resulting flow is then validated byPauliFlow.check_well_formed.Correctness
The decisive check is the round trip: for the reconstructed
pf,pf.to_corrections()must reproduce the pattern's X- and Z-corrections exactly (this is what guarantees it generates this pattern). Tests verify well-formedness and the round trip on:p(0) = {1, 3}, p(1) = {2}, p(2) = {3}etc., including the anachronical node1),There are also unit tests for the GF(2) solver. Passes
ruff,mypy --strict,pyright, andpytestlocally (new tests plus the existing flow / open-graph / pattern suites).Developed with LLM assistance, reviewed and tested by me.