Adding gate decomposition #103
Conversation
There was a problem hiding this comment.
Using transpile looks to be going in a good direction but it is a bit overkill. transpile transforms the circuit in many ways and we just want to use a single functionality. As you notice in your PR header, you end up with a qubit mapping bug due to #50.
It would be great to extract the bare minimum from transpile and use it efficient to decompose the circuit.
One thing I particularly like is that you would be able to decompose in specific gate sets such as backends' native gate set. It would be interesting to see if you could extend your PR in this direction.
|
|
||
| def transpile(circuit): | ||
| """Transpiles a Qiskit circuit into a circuit whose gates are accepted by Braket.""" | ||
| braket_gates = [ |
There was a problem hiding this comment.
It would be great to retrieve all these gates from the backend (i.e. from the device capabilities).
| } | ||
|
|
||
| for gate_name in qiskit_gates: | ||
| params = self._generate_params( |
There was a problem hiding this comment.
Nit: If in a new release, qiskit adds a new gate, this will likely fail as it won't provide parameters for this new gate. We could create a test for this, hardcode the list of gates to test or let it as it is since it will warn us that we need to extend the test to this new gate.
| ).__init__.__code__.co_varnames, | ||
| ) | ||
| try: | ||
| qiskit_gates[gate_name] = getattr(qiskit.circuit.library, gate_name)( |
There was a problem hiding this comment.
A try-except in a test does not sound safe. Why do we need this? What will we miss if a error occurs but it undetected.
| bind_dict[key] = np.random.random() | ||
| source_circuit = source_circuit.bind_parameters(bind_dict) | ||
| transpiled_circuit = transpile(source_circuit) | ||
| local_simulator.run(transpiled_circuit, shots=10).result().get_counts() |
There was a problem hiding this comment.
This does not insure that it would work on all backends. Could we check the circuit itself? Does it have only gates from a specific subsets? Does it have the same qubit register?
| source_circuit = QuantumCircuit(1) | ||
| source_circuit.prepare_state([1 / np.sqrt(2), -1 / np.sqrt(2)], 0) | ||
| transpiled_circuit = transpile(source_circuit) | ||
| local_simulator.run(transpiled_circuit, shots=10).result().get_counts() |
There was a problem hiding this comment.
Why not testing the validity of the output?
| evo = PauliEvolutionGate(operator, time=0.2) | ||
| source_circuit = SuzukiTrotter().synthesize(evo) | ||
| transpiled_circuit = transpile(source_circuit) | ||
| local_simulator.run(transpiled_circuit, shots=10).result().get_counts() |
There was a problem hiding this comment.
Why not testing the validity of the output? Do we get the right result?
|
After a second thought, I think the use of |
Summary
I have implemented a
transpilefunction which aim at resolving the issue #90.Details and comments
As of now, the
resetgate is failing to transpile and I am getting someNon-contiguous qubit indices supplied; qubit indices in a circuit must be contiguous.for some other gates (PiecewisePolynomialPauliRotations,XOR). Apparently, this last error is due to controlled-gates between non-connected qubits and is specific to some device and to the simulator. I am not sure if I should or should not try to resolve this issue as it is hardware-specific. I opened this draft PR to have feedback on my progress so far and to have recommendation on how to continue or if there is any further modifications or improvements you would like to see.