Adding filter_fn argument to Collect1qRuns and Collect2qBlocks

[alexanderivrii]

Summary

This PR adds a new argument filter_fn to python-space single-qubit/two-qubit collection passes Collect1qRuns and Collect2qBlocks. This argument can be used to filter collected subcircuits based on custom criteria, such as "only return 2-qubit blocks with at least 3 CX-gates", or "only return 1-qubit runs with no Hadamard gates".

Note that this is different from the filtering mechanisms used within DAGCircuit.collect_1q_runs and DAGCircuit.collect_2q_blocks, which specify which nodes should be included in runs/blocks in the first place. The new additional filtering mechanism filters entire runs/blocks.

I personally need this for improving the Clifford+T transpiler pipeline, where we want to create a PassManager that collects 1-qubit runs, but avoids resynthesizing runs that already consist entirely of Clifford+T gates. Note that this is not a part of this PR.

Details and comments

Even though the immediate use-cases filter the runs just based on the number or the types of gates they contain, the filtering function also takes the DAGCircuit as an argument, as I think this could be useful for more complex filtering strategies. But I don't feel too strongly about this, and can remove DAGCircuit to keep the signatures simpler.

This is currently implemented purely in python space because I don't think we will get much of a performance gain from passing a python callback to Rust. Additionally, it would be better to wait with the Rust implementation until #15301 is merged.

[qiskit-bot]

One or more of the following people are relevant to this code:

• @Qiskit/terra-core

[coveralls]

Pull Request Test Coverage Report for Build 21468458880

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 18 of 18 (100.0%) changed or added relevant lines in 2 files are covered.
• 84 unchanged lines in 3 files lost coverage.
• Overall coverage decreased (-0.01%) to 87.966%

---

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	5	91.77%
crates/qasm2/src/parse.rs	12	97.15%
qiskit/circuit/quantumcircuit.py	67	94.36%

Totals
Change from base Build 21432860966:	-0.01%
Covered Lines:	100024
Relevant Lines:	113708

---

💛 - Coveralls

[jakelishman]

Taken at face value, the code in this PR is fine. On an API question, though: is this is better on Collect1qRuns, or should it be logic that's in the subsequent consumer of it? Having it here doesn't affect the actual collection of a run at all, so it can be done afterwards without a performance penalty. Or is there an API reason that it the logic about whether to use a collected run or not should be a function of the producer, not the consumer?

What does the full pipeline look like where you'd use this?

[jakelishman]

Now we're on Python 3.10, I don't think you need the __future__ import, and you can just import DAGCircuit, DAGOpNode without the guard because pylint shouldn't complain any more (but not 100% certain).

[alexanderivrii]

Thanks Jake, trying out your suggestion in 24bd9aa. But it does already work locally for me (with python 3.13).

[alexanderivrii]

Hmm, this seems to have worked for all of our platforms/python versions.

[alexanderivrii]

Jake: I agree that I could have equally well added the logic to ConsolidateBlocks (currently the only consumer of the runs). Other possibilities include adding the logic to both, or implementing this as a standalone analysis pass.

Personally I am slightly leaning towards having this logic on the producer side, because internally the rust functions for collecting runs/blocks also depend on include_node_fn, so in principle having the two collection functions have access to both "which nodes are eligible to be included in a sequence?" and "which sequences we should avoid collecting?" does make a bit of sense. I was brainstorming with Luciano on this earlier, and in principle in some cases with this information available we can do better collecting and/or early termination. However, none of this is implemented right now, and if you have a stronger opinion that this logic should be on the consumer side, I will be happy to move the code.

[alexanderivrii]

Superseded by #15625.