Integrate qiskit-toqm as an optional package for layout and routing.

[kevinhartman]

Summary

Integrates the ToqmSwap layout and routing pass from Qiskit TOQM via an optional package for all preset pass managers.

Details and comments

Qiskit TOQM is implemented on top of libtoqm, a C++ library-ified version of the original reference implementation published with Time-Optimial Qubit Mapping.

The integration points aim to be minimal in support of the somewhat distant goal of the transpiler being pluggable. For now, the goal here is to make TOQM easily accessible to users and easy to update without locking it against Terra's release cadence.

Because there's not yet been enough experimentation to deduce the best TOQM configurations for each preset pass manager, qiskit-toqm exposes "strategies" for optimization levels (O1, O2, O3) maintained within. As we determine the appropriate settings for Terra, we can easily update the behavior by modifying those strategies. Users that desire more control can construct their own pass manager and specify a custom strategy callable which offers complete control over the underlying invocation of libtoqm with access to information about the current DAG and target.

TODO:

• qiskit-toqm is not yet available in PyPI.
• Needs release note.

[kevinhartman]

Opening as a draft for now. It'd be great to get some early feedback on the approach, since it'd be nice to make TOQM available as part of the 0.20 release!

[coveralls]

Pull Request Test Coverage Report for Build 2496692372

• 38 of 41 (92.68%) changed or added relevant lines in 5 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.02%) to 84.441%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/transpiler/preset_passmanagers/level0.py	9	10	90.0%
qiskit/transpiler/preset_passmanagers/level2.py	9	10	90.0%
qiskit/transpiler/preset_passmanagers/level3.py	9	10	90.0%

Totals
Change from base Build 2495685731:	0.02%
Covered Lines:	54760
Relevant Lines:	64850

---

💛 - Coveralls

[ajavadia]

The approach looks good, a few comments:

• can you add some more docstring under ToqmStrategyO1, ToqmStrategyO2, ToqmStrategyO3 to describe what kind of parameters you are changing in each of them (in the qiskit-toqm repo)?

• please modify the docstring of the transpile() function to add 'toqm' to the list of possible layout_methods and routing_methods.

• can TOQM be viewed as a scheduling_method too? Recently in Split scheduling pass into scheduling and padding #7709 the scheduling methods (currently only ASAP and ALAP) have been made such that they populate a node_start_time dictionary in the property set, which essentially puts absolute start time on the nodes. A follow up "padding" pass will actually insert the delays between those nodes. I wonder if TOQM can also populate node_start_time within it? Or are the timings too rough to do that?

[ajavadia]

does this need some tests? also should it be added as an optional dependency?

[kevinhartman]

I'm planning to make a few changes to the qiskit-toqm API after discovering that layout is not configurable when using non-optimal configurations. I'll take this off of hold once it's been updated to reflect the new API.

[kevinhartman]

@ajavadia and I have discussed some of this offline, but for others:

can you add some more docstring under ToqmStrategyO1, ToqmStrategyO2, ToqmStrategyO3 to describe what kind of parameters you are changing in each of them (in the qiskit-toqm repo)?

From an API perspective, the details of what's being changed behind these opt level-based strategies should be somewhat opaque, since we want the flexibility to change the behavior in future qiskit-toqm updates. The actual intent of this portion of the interface is something like: in general, higher optimization levels should mean a shorter overall circuit duration, at the expense of more compilation time. Right now, this isn't strictly true, in that it's possible for a lower level to produce a shorter circuit, or to take longer than a higher level—it's just unlikely. We could make the relationship strictly true by always running the circuit with the requested opt level and all levels below it, returning the routed circuit with the shortest duration. Let me know if you have an opinion there.

If users want to control specific parameters, they can create their own ToqmStrategy like this, modifying any of the parameters used to construct native class toqm.ToqmMapper:

from qiskit_toqm import ToqmSwap, ToqmStrategy
import qiskit_toqm.native as toqm

class CustomStrategy(ToqmStrategy):
    def run(self, gates, num_qubits):
        mapper = toqm.ToqmMapper(
            toqm.TrimSlowNodes(...),
            toqm.GreedyTopK(...),
            toqm.CXFrontier(),
            toqm.Latency_1_2_6(),
            [toqm.GreedyMapper()],
            [],
            0
        )
        mapper.setRetainPopped(1)
        return mapper.run(gates, num_qubits, self.coupling_map)

We can certainly improve the documentation of the native layer to make customization easier.

please modify the docstring of the transpile() function to add 'toqm' to the list of possible layout_methods and routing_methods.

Done in 6f81c34.

can TOQM be viewed as a scheduling_method too? Recently in #7709 the scheduling methods (currently only ASAP and ALAP) have been made such that they populate a node_start_time dictionary in the property set, which essentially puts absolute start time on the nodes. A follow up "padding" pass will actually insert the delays between those nodes. I wonder if TOQM can also populate node_start_time within it? Or are the timings too rough to do that?

TOQM cannot do this currently, since it works in terms of "cycles". These are different yet from dt, since TOQM does work for every cycle from the start of the circuit (and so cycles == dt would mean unnecessarily long execution times).ToqmStrategys are also free to estimate gate durations (e.g. Latency_1_2_6), so it'd be complicated. Unless we rewrite libtoqm from scratch, let's just keep this as a layout/routing pass.

does this need some tests?

Testing added in c1bcec9. This testing aims only to cover the integration points with Terra, rather than the correctness of resulting circuits. This is because we don't want updates to qiskit-toqm to break Terra, and it seems to make more sense for implementation testing to live in the same repo as the pass. Once this PR merges, we can merge qiskit-community/qiskit-toqm#17, which will enable nightly circuit equivalence testing for qiskit-toqm.

also should it be added as an optional dependency?

Is there something you have in mind other than what's been done already (i.e. adding HAS_TOQM to optionals)?

[jakelishman]

About the very last point: you could potentially add an extra in setup.py that lets you do (e.g.) pip install qiskit-terra[toqm] if you like as well.

[ajavadia]

If users want to control specific parameters, they can create their own ToqmStrategy like this, modifying any of the parameters used to construct native class toqm.ToqmMapper:

from qiskit_toqm import ToqmSwap, ToqmStrategy
import qiskit_toqm.native as toqm

class CustomStrategy(ToqmStrategy):
    def run(self, gates, num_qubits):
        mapper = toqm.ToqmMapper(
            toqm.TrimSlowNodes(...),
            toqm.GreedyTopK(...),
            toqm.CXFrontier(),
            toqm.Latency_1_2_6(),
            [toqm.GreedyMapper()],
            [],
            0
        )
        mapper.setRetainPopped(1)
        return mapper.run(gates, num_qubits, self.coupling_map)

I guess I'm fine if ToqmStrategy wraps all the options. But can't it be simplified a little? Like can't it just be something like

from qiskit_toqm import ToqmStrategy
custom_strategy = ToqmStrategy(trim_slow_nodes=..., greedy_top_k=..., cx_frontier=...)

and then ToqmStrategyO1 becomes a singleton instance of ToqmStrategy.

Is the class with the run() method offering anything that this doesn't?

[ajavadia]

We could make the relationship strictly true by always running the circuit with the requested opt level and all levels below it, returning the routed circuit with the shortest duration. Let me know if you have an opinion there.

No I don't think we need to guarantee this. Just something that on average is true is fine (higher optimization level, more time, better output)

[kevinhartman]

I guess I'm fine if ToqmStrategy wraps all the options. But can't it be simplified a little?

It sounds like you're wondering if the ToqmStrategy you pass to ToqmSwap can just be a callable.

The class with run offers a separation between initialization (on_pass_init) and run. During pass init, the strategy gets information specific to the transpilation computed by ToqmSwap, i.e. the cycle-based instruction durations.

But now that I think about it a bit more, there's a level of indirection there we don't need, since we have all the information necessary to calculate instruction durations while constructing the pass manager. Further, cycle-based instruction durations should really only be computed if the strategy will actually use them.

I'll try refactoring the ToqmSwap and ToqmStrategy interface to more closely match the usage you're describing.

[kevinhartman]

After looking into the above a bit more, I think we should stick with the current interface. While we do have a level of indirection that isn't strictly necessary, ToqmSwap was originally expected to always consider the backend's instruction durations.

As we have it now, the optimization strategies express more than just a native ToqmMapper instance. They currently choose an approach (optimal or non-optimal) based on the size of the target, and could in theory adjust the mapper's settings based on other properties of the incoming DAG, such as the number of gates.

Instead, I'll add a strategy to qiskit-toqm that lines up directly with the native ToqmMapper interface to make it easier to experiment.

[kevinhartman]

I ended up doing a bit of refactoring in qiskit-community/qiskit-toqm#23.

That refactor moves all of the target-specific latency calculations out of ToqmSwap to a helper function latencies_from_target. This is a much better factoring, since really ToqmSwap shouldn't be concerned with latencies at all—it's up to the selected strategy whether or not latencies are applicable.

As a side-effect, this allowed us to simplify the strategy hierarchy. The interface ToqmStrategy has been removed in favor of using a simple callable object. We still have ToqmStrategyOX for each optimization level, but these are themselves just callables that select either a ToqmOptimalStrategy or ToqmHeuristicStrategy callable, based on the device and circuit.

When constructing your own PassManager, it's now easy to parameterize the underlying libtoqm invocation by using ToqmOptimalStrategy or ToqmHeuristicStrategy directly.

For example, to create a ToqmSwap pass that will use a heuristic strategy with a particular libtoqm configuration from a Terra target, you can do something like:

from qiskit_toqm import ToqmSwap, ToqmHeuristicStrategy, latencies_from_target

latencies = latencies_from_target(coupling_map, instruction_durations, basis_gates, backend_properties)

# or if you're using a V2 backend:
latencies = latencies_from_target(backend.target)

strategy = ToqmHeuristicStrategy(latencies, top_k=10, queue_target=1000, queue_max=2000)
swap = ToqmSwap(coupling_map, strategy)

If you want to hard-code latencies rather than calculate them given a Terra target, you can use latencies_from_simple instead, as shown in the next example.

To create an optimal ToqmSwap pass from hard-coded cycles, do something like:

from qiskit_toqm import ToqmSwap, ToqmOptimalStrategy, latencies_from_simple

latencies = latencies_from_simple(one_qubit_cycles=1, two_qubit_cycles=2, swap_cycles=6)
strategy = ToqmOptimalStrategy(latencies, perform_layout=False)
swap = ToqmSwap(coupling_map, strategy)

Note that perform_layout is only available for ToqmOptimalStrategy, since the heuristic configurations do not support disabling it.

[kevinhartman]

Just want to point out as well that since this PR includes testing of the qiskit-toqm integration points, it is possible to break Terra's CI by pushing a broken change to qiskit-toqm. I'm curious to know what others think about this.

To avoid this, I think our options would be either:

1. Omit testing of the TOQM integration from Terra entirely. Instead, we could move the testing from this PR to the qiskit-toqm repo. I think this is the more natural approach, since this is how things would work in a fully pluggable transpiler archiecture.
2. Keep the testing, but tie down the qiskit-toqm version in requirements-dev.txt. The downside here is that we need to do some work to pick up qiskit-toqm changes, even if they're not API-breaking.

[mtreinish]

We should ensure that we've added an extras entry in the setup.py for the qiskit-toqm package. For example see: https://github.com/Qiskit/qiskit-terra/blob/main/setup.py#L48-L51 and https://github.com/Qiskit/qiskit-terra/blob/main/setup.py#L84-L93 for where we do this for other optional dependencies.

[mtreinish]

Just want to point out as well that since this PR includes testing of the qiskit-toqm integration points, it is possible to break Terra's CI by pushing a broken change to qiskit-toqm. I'm curious to know what others think about this.

To avoid this, I think our options would be either:

Omit testing of the TOQM integration from Terra entirely. Instead, we could move the testing from this PR to the qiskit-toqm repo. I think this is the more natural approach, since this is how things would work in a fully pluggable transpiler archiecture.
Keep the testing, but tie down the qiskit-toqm version in requirements-dev.txt. The downside here is that we need to do some work to pick up qiskit-toqm changes, even if they're not API-breaking.

This honestly isn't any different then any other upstream dependency for testing (except that the responsibility is on us instead of others). If we release a qiskit-toqm package that breaks terra we'd have to pin the version until the breaking change is fixed. This happens semi-regularly with many of our dependencies that don't have good test coverage. If there is going to be a toqm usage in the terra repository it will need to be tested here, but we should also try to ensure that the qiskit-toqm package has sufficient test coverage to ensure that proposed changes to the qiskit-toqm won't break terra's usage.

But if you're saying that the qiskit-toqm package doesn't provide sufficient api stability guarantees for us to rely on it that's a different story and maybe we shouldn't be integrating it into the terra repo yet until we've locked down what the api will look like.

[kevinhartman]

We should ensure that we've added an extras entry in the setup.py for the qiskit-toqm package.

Ah, I see now what you and @jakelishman were getting at. I'll add this in an additional commit.

If there is going to be a toqm usage in the terra repository we should try to ensure that the qiskit-toqm package has sufficient test coverage to ensure that proposed changes to the qiskit-toqm won't break terra's usage.

I agree with this. I think the best plan is to move the tests added in this PR to qiskit-toqm, and tie qiskit-toqm's CI against Terra's main branch. Once the integration points with transpile from this PR merge, I can merge those tests to qiskit-toqm. Changes to Terra will be able to break qiskit-toqm, but that should be relatively easy to deal with.

*EDITED*

If there is going to be a toqm usage in the terra repository it will need to be tested here, but we should also try to ensure that the qiskit-toqm package has sufficient test coverage to ensure that proposed changes to the qiskit-toqm won't break terra's usage.

This is fine with me. I think it's valuable to exercise the codepath that's hit when specifying routing_method="toqm", though I'm less certain that it's Terra's responsibility to test the correctness of the qiskit-toqm implementation.

But if you're saying that the qiskit-toqm package doesn't provide sufficient api stability guarantees for us to rely on it that's a different story and maybe we shouldn't be integrating it into the terra repo yet until we've locked down what the api will look like.

I'm happy with the current qiskit-toqm API, though I suppose I should make it "target aware", in the sense that latencies_from_target should probably also take a Target object and override the other backend-related parameters, as we do with other passes currently. Otherwise, it's ready to go. UPDATE: qiskit-toqm is now target-aware / backend V2 ready.

[kevinhartman]

Should be ready for another look @ajavadia.

[ajavadia]

I like the new interface. Thanks for doing the refactoring. I'm good to merge after the conflict is resolved.

Are the usage examples you have posted above for how to build strategies and latencies documented in qiskit_toqm?

[kevinhartman]

@ajavadia IIRC, I do need to update the docs in the qiskit-toqm repo. I'll plan to do this next week while I prep to show this off for the Qiskit demo day.