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

• @Qiskit/terra-core

Pull Request Test Coverage Report for Build 13070354287

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

• 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
• 1488 unchanged lines in 47 files lost coverage.
• Overall coverage decreased (-0.1%) to 88.814%

Totals
Change from base Build 12835664036:	-0.1%
Covered Lines:	79759
Relevant Lines:	89805

💛 - Coveralls

I agree it should leave Measure and Reset as is, but I'm not sure if we should just ignore gates that don't have to_matrix. I always thought of SK like a kind of basis translator that takes you to the target 1q basis and, like BasisTranslator, it would fail if it can't 🤔

I agree it should leave Measure and Reset as is, but I'm not sure if we should just ignore gates that don't have to_matrix. I always thought of SK like a kind of basis translator that takes you to the target 1q basis and, like BasisTranslator, it would fail if it can't 🤔

This is indeed a valid alternative. I like the analogy with the BasisTranslator pass, though (unlike BT) SK already ignores all operations on 2 or more qubits, and it feels strange that the pass should ignore, for instance, the 2-qubit initialize instruction while throw an error for the 1-qubit initialize instruction.

The other thing is that probably we need to make the pass recursive for control-flow ops (and not just ignore these as it happens right now), what do you think?

Maybe we should first decide on the typical usages of the SK pass, when and how it would be used in a typical transpiler pipeline.

The SK algorithm only applies to decompositions of $U(2)$, so it makes sense to ignore anything that isn't in that. Under that rule, ignoring both Measure and any Initialize is consistent, because neither is unitary.

Agree that the pass should recurse over control-flow operations.

If we're intending to use the SK algorithm as part of a translation stage, we might want to consider it as an entire pipeline. To use the BasisTranslator example: that alone isn't an entire translation stage; the stage also includes Unroll3qOrMore. If we want to make an SK-based translation plugin, I'd suggest we make an entire plugin, rather than trying to make the pass try to guess - the pipeline can be something like

• unroll 3q or more
• consolidate blocks
• unitary synthesis (min_qubits=2) to a target basis [u, <2q gates>]
• SK

In an ideal world, we'd want UnitarySynthesis to output the 1q unitaries without bothering to find their Euler-angle decomposition, but we could potentially work out how to improve that story later.

I wouldn't necessarily make the SK pass error out if it can't translate, since it's conceivable to me that you might want to use it in other contexts - we can make the translation stage error out in that case, potentially, where it's clear that the stage is required to fail if translation can't be done.

Thanks @jakelishman, @Cryoris! I have added recursion for control-flow operators. If we agree that the SK pass should not error out if it can't translate, is there more you suggest to do here, modulo correcting the class documentation and polishing the release notes?

Let's take the discussion of the transpilation pipeline to #13695.