Reimplement flow matching using Gaussian elimination#45
Conversation
…into enhance/flow-matching
smburdick
left a comment
There was a problem hiding this comment.
Mostly documentation related.
|
|
||
|
|
||
| def _solve_linear_system( | ||
| basis: dict[int, tuple[int, int]], x: int, update_basis: bool = True |
There was a problem hiding this comment.
Might want to define a Basis type or class somewhere, just to make it clear what the valid keys/values are, kind of like what's in tqec.
| """Try to find a set of boundary stabilizers from ``sources`` that generate a | ||
| superset of ``target``. | ||
|
|
||
| This function try to find a set of Pauli strings from ``sources`` that |
There was a problem hiding this comment.
typo: "This function tries to find ..."
| superset of ``target``. | ||
|
|
||
| This function try to find a set of Pauli strings from ``sources`` that | ||
| includes ``target`` (i.e., on every qubit where ``target`` is non-trivial, |
There was a problem hiding this comment.
What is a trivial target?
| the product of each of the returned Pauli strings should commute with | ||
| ``target``). | ||
|
|
||
| The differences with :func:`find_cover` are: |
There was a problem hiding this comment.
Would find_noncommuting_cover be a valid name for find_cover?
There was a problem hiding this comment.
The method names are somewhat legacy from the previous implementation. Now after simplification, it's clear that they only differ by how they treat the involved qubits and edge cases. I will think about whether the naming could be improved.
|
|
||
| If multiple valid covers exist, the covers involving the lowest number of | ||
| :class:`~tqecd.pauli.PauliString` instances from ``sources`` are listed, and | ||
| a random cover is picked from that list. |
There was a problem hiding this comment.
Why not just return all of the covers?
There was a problem hiding this comment.
Because there are exponentially many of them, and they don't really make a difference. But this doc describes the previous behavior and is inaccurate now. Will fix.
| def find_exact_cover( | ||
| target: PauliString, sources: list[PauliString] | ||
| ) -> list[int] | None: | ||
| """Try to find a set of pauli strings from ``sources`` that generate exactly |
| """Try to find a set of pauli strings from ``sources`` that generate exactly | ||
| ``target``. | ||
|
|
||
| The Pauli strings returned (via indices over the provided ``sources``), once |
There was a problem hiding this comment.
By the by, I noticed #47 , are you feeling the slowdown in this computation too?
There was a problem hiding this comment.
This computation doesn't rely on the PauliString class too much, because Pauli strings are converted to ints. There could be a speedup for the conversion.
| @@ -1,15 +1,11 @@ | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
Do you think it's time to start moving the test files into their own directory
There was a problem hiding this comment.
Not very critical, but yeah, something we can do.
| [project] | ||
| name = "tqecd" | ||
| version = "0.1.3" | ||
| version = "0.1.4" |
There was a problem hiding this comment.
Do we want to do a release?
There was a problem hiding this comment.
Yes. Notice the reduced dependencies and the further implications.
| "numpy>=1.26; python_version >= '3.12'", | ||
| "numpy>=2.1; python_version >= '3.13'", | ||
| "stim>=1.15.0", | ||
| ] |
There was a problem hiding this comment.
Is dependabot needed for this repo?
There was a problem hiding this comment.
I think there are some bots already? But admittedly, the CICD aspect is not as polished as tqec. Let's start by switching to uv, ty, and ruff and make the pyproject and GitHub action configurations more rigorous.
|
This PR is superseded by #51. I will fix the typos, etc. there. |
This is a local version of #45 to avoid GitHub action permission issues when merging from a fork. --------- Co-authored-by: Copilot <copilot@github.com>
Pending tqec/tqecd#45. Bump tqecd dependency version; remove SAT solver dependencies; fix a typo in pyproject.toml.
Flow matching is a major bottleneck of tqecd's performance and scalability. The matchings are solved by (SAT-solver-accelerated) brute force in main. This PR reimplements the functions in match_utils/cover.py to solve them efficiently using Gaussian elimination over GF(2), which partly addresses #2.
The solution's minimal-count requirement needed to be relaxed, but I don't think it fundamentally affects anything. All tqecd tests pass, and all tqec tests (including tests marked as slow) pass with
do_not_use_database=True.Benchmarking using a script adapted from tqec/tqec#356 (comment) shows that both the constant factor and scalability are drastically improved:
This PR:
main:
Note that these improvements will not lead to a significant speedup in the detector annotation of tqec's ZX cubes, as they use the detector database and subtemplates to confine detector findings to small subcircuits. However, it is expected to benefit circuits that lack a nice subtemplate partitioning, such as cultivation and Y initialization/measurement. They should also benefit #37.
A bonus is that the SAT solver dependencies can be removed (included in this PR), making tqecd easily installable on platforms without a precompiled binary.
The int-based efficient Gaussian elimination infrastructure is borrowed from my implementation in https://github.com/tqec/tqec/blob/main/src/tqec/computation/_correlation.py.