-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pam verify bug fixes #192
Pam verify bug fixes #192
Changes from all commits
ae1dffa
d7ae7cb
d020a20
26a5025
4e0f8e0
2d501a5
10b2c18
a803bde
a5a224d
b94ce8f
a266443
89fac09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
"""This module implements the GeneralizedSabreRoutingPass class.""" | ||
from __future__ import annotations | ||
|
||
import copy | ||
import logging | ||
|
||
from bqskit.compiler.basepass import BasePass | ||
|
@@ -22,16 +21,12 @@ class GeneralizedSabreRoutingPass(BasePass, GeneralizedSabreAlgorithm): | |
|
||
async def run(self, circuit: Circuit, data: PassData) -> None: | ||
"""Perform the pass's operation, see :class:`BasePass` for more.""" | ||
subgraph = self.get_connectivity(circuit, data) | ||
subgraph = data.connectivity | ||
if not subgraph.is_fully_connected(): | ||
raise RuntimeError('Cannot route circuit on disconnected qudits.') | ||
|
||
if 'initial_mapping' in data: | ||
pi = copy.deepcopy(data['initial_mapping']) | ||
else: | ||
pi = [i for i in range(circuit.num_qudits)] | ||
|
||
pi = [i for i in range(circuit.num_qudits)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So is the initial mapping always going to 0,1,2,3,... and all of the actual mapping information is in placement? Do we even need initial mapping anymore? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the initial mapping gets updated when we actually apply the placement. Until then, the circuit's input qubit ordering actually doesn't change |
||
self.forward_pass(circuit, pi, subgraph, modify_circuit=True) | ||
# TODO: if final_mapping is already in data, apply it first | ||
data['final_mapping'] = pi | ||
data.final_mapping = [pi[x] for x in data.final_mapping] | ||
|
||
_logger.info(f'Finished routing with layout: {str(pi)}') |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,60 @@ | ||
from __future__ import annotations | ||
|
||
from typing import Callable | ||
|
||
import pytest | ||
|
||
from bqskit.compiler.compile import compile | ||
from bqskit.compiler.compiler import Compiler | ||
from bqskit.compiler.machine import MachineModel | ||
from bqskit.ir.circuit import Circuit | ||
from bqskit.qis.graph import CouplingGraph | ||
from bqskit.qis.permutation import PermutationMatrix | ||
|
||
|
||
def default_model_gen(circuit: Circuit) -> MachineModel: | ||
"""Generate a default model for the given circuit.""" | ||
return MachineModel(circuit.num_qudits) | ||
|
||
|
||
def linear_model_gen(circuit: Circuit) -> MachineModel: | ||
"""Generate a linear model for the given circuit.""" | ||
return MachineModel( | ||
circuit.num_qudits, | ||
CouplingGraph.linear(circuit.num_qudits), | ||
) | ||
|
||
|
||
@pytest.mark.parametrize( | ||
'gen_model', | ||
[ | ||
default_model_gen, | ||
linear_model_gen, | ||
], | ||
ids=[ | ||
'default', | ||
'linear', | ||
], | ||
) | ||
def test_medium_circuit_compile( | ||
compiler: Compiler, | ||
optimization_level: int, | ||
medium_qasm_file: str, | ||
gen_model: Callable[[Circuit], MachineModel], | ||
) -> None: | ||
circuit = Circuit.from_file(medium_qasm_file) | ||
model = gen_model(circuit) | ||
out_circuit, pi, pf = compile( | ||
circuit, | ||
optimization_level=optimization_level, | ||
with_mapping=True, | ||
compiler=compiler, | ||
model=model, | ||
) | ||
in_utry = circuit.get_unitary() | ||
out_utry = out_circuit.get_unitary() | ||
PI = PermutationMatrix.from_qubit_location(out_circuit.num_qudits, pi) | ||
PF = PermutationMatrix.from_qubit_location(out_circuit.num_qudits, pf) | ||
error = out_utry.get_distance_from(PF.T @ in_utry @ PI, 1) | ||
assert error <= 1e-8 | ||
assert model.is_compatible(out_circuit) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
from bqskit.passes import GeneralizedSabreRoutingPass | ||
from bqskit.passes import GreedyPlacementPass | ||
from bqskit.passes import SetModelPass | ||
from bqskit.passes.mapping.apply import ApplyPlacement | ||
from bqskit.qis import PermutationMatrix | ||
from bqskit.qis.unitary.unitarymatrix import UnitaryMatrix | ||
|
||
|
@@ -38,12 +39,17 @@ def test_simple(compiler: Compiler) -> None: | |
GreedyPlacementPass(), | ||
GeneralizedSabreLayoutPass(), | ||
GeneralizedSabreRoutingPass(), | ||
ApplyPlacement(), | ||
] | ||
|
||
cc, data = compiler.compile(circuit, workflow, True) | ||
pi = data['initial_mapping'] | ||
pf = data['final_mapping'] | ||
PI = PermutationMatrix.from_qubit_location(5, pi) | ||
PF = PermutationMatrix.from_qubit_location(5, pf) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How did some qudits become inactive? Not sure I understand There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you apply the placement, the logical, input circuit size is extended to fit the physical, output machine model. So in this workflow, this means a good portion of them won't be used. |
||
inactive = [i for i in range(cc.num_qudits) if i not in cc.active_qudits] | ||
inactive.sort(reverse=True) | ||
for i in inactive: | ||
cc.pop_qudit(i) | ||
assert cc.get_unitary().get_distance_from(PF.T @ in_utry @ PI) < 1e-7 | ||
assert all(e in cg for e in cc.coupling_graph) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pi is the permutation of the initial mapping we are solving for? So it gets modified by the forward and backwards passes? Might be helpful to comment this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout is also called after ApplyPlacement correct? So what good does changing the placement do here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, ok, in the test_sabre I see that ApplyPlacement is called last, do we need to change this in other workflows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good questions. A few points:
circuit
qudits tomodel
qudits, so layout and routing need to start with this informationdata.connectivity
call, the actual coupling graph is permuted there to account for itYes, ApplyPlacement should be called after SABRE layout and routing. What other workflows are you referring to?
Also, Rather than running
GreedyPlacement
, another valid placement strategy would be toApplyPlacement
before mapping to "extend" the circuit to the model size, then running layout and hoping SABRE finds a good placement. This is sort of the strategy we take with PAM. We are definitely lacking in the placement department thoThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok, so even before we were calling ApplyPlacement before and after PAMLayout and PAMRouting.