Optionally reuse flows for anti-commuting covers#37
Conversation
|
A preview of e50278f is uploaded and can be seen here: ✨ https://tqec.github.io/tqecd/pull/37/ ✨ Changes may take a few minutes to propagate. |
HaoTy
left a comment
There was a problem hiding this comment.
Will try to review this PR over the weekend. Once we make the performance improvements to cover finding and the Pauli string, the overhead should be largely mitigated.
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in mode to the detector-finding pipeline that allows reusing stabilizer flows when forming commuting stabilizers from anti-commuting ones, enabling additional detector extraction in cases where the previous “consume all used flows” behavior prevented finding complete detector sets.
Changes:
- Add
reuse_flows_for_anticommuting_coveroption to the publicannotate_detectors_automaticallyAPI and propagate it through fragment compilation and detector matching. - Extend flow merging (
try_merge_anticommuting_flows) to optionally remove only one contributing flow per merge instead of all contributing flows. - Add a regression test covering the reported circuit and verifying the detector count difference with/without reuse enabled.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/tqecd/match.py |
Threads the new reuse option into boundary stabilizer matching and anti-commuting merge step. |
src/tqecd/flow.py |
Adds reuse_flows parameter to anti-commuting merge logic and modifies removal behavior. |
src/tqecd/construction.py |
Exposes the option on annotate_detectors_automatically and propagates it through compilation (including loop recursion). |
src/tqecd/construction_test.py |
Adds a regression test for the reported circuit behavior under the new option. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Update flows: remove one entry per merged stabilizer. | ||
| # This ensures: | ||
| # 1. A stabilizer isn't merged twice. | ||
| # 2. Flows remain valid for finding subsequent merging opportunities, | ||
| # as the removed flow's anti-commuting boundary stabilizer is covered | ||
| # by the remaining flows. | ||
| # Note that we have the risk that the reused flows may have unnecessarily | ||
| # many measurements included in the formed detector. However, we never | ||
| # guarantee minimality of detector structures, so this is not an issue. | ||
| if reuse_flows: | ||
| flows.pop(flows_indices_of_stabilizers_to_merge[-1]) | ||
| else: | ||
| for i in sorted(flows_indices_of_stabilizers_to_merge, reverse=True): | ||
| flows.pop(i) |
| assert annotate_detectors_automatically(circuit).num_detectors == 1 | ||
| assert ( | ||
| annotate_detectors_automatically( | ||
| circuit, reuse_flows_for_anticommuting_cover=True | ||
| ).num_detectors | ||
| == 2 | ||
| ) |
|
My apologies for the delay! With the Gaussian-elimination-based flow matching currently in main, the added test fails due to the flow order being different from the SAT-based matching. |
This issue is found by one of my colleagues (Weiping Lin).
For the following circuit:
There should be two detectors
DETECTOR rec[-1]andDETECTOR rec[-2]. However,annotate_detectors_automaticallyreturns a single detectorDETECTOR rec[-1] rec[-2]. This is because intqecd/src/tqecd/flow.py
Lines 90 to 93 in 7e47c39
This PR adds an option to reuse the flows for further anticommuting cover opportunities. It's achieved by removing a single flow instead of all the flows each time. Note that it will significantly increase the runtime when this option is on but it enables more opportunities to find complete detectors.