qiskit-community / qiskit-nature

Qiskit Nature is an open-source, quantum computing, framework for solving quantum mechanical natural science problems.
https://qiskit-community.github.io/qiskit-nature/
Apache License 2.0
301 stars 204 forks source link

transpiling UCCSD with unbound parameters fails unless num_parameters, parameters, or num_qubit class attributes are called first #671

Closed JoelHBierman closed 2 years ago

JoelHBierman commented 2 years ago

Environment

What is happening?

Calling transpile(UCCSD_instance, backend=some_backend) fails unless certain class attributes are first called. (e.g. I have tested num_parameters, parameters, and num_qubits, but there may be others.)

How can we reproduce the issue?

Running the following code:

from qiskit import transpile
from qiskit_nature.converters.second_quantization import QubitConverter
from qiskit_nature.mappers.second_quantization import JordanWignerMapper
from qiskit_nature.circuit.library import UCCSD, HartreeFock
from qiskit.opflow import AerPauliExpectation
from qiskit.providers.aer import AerSimulator
from qiskit.utils import QuantumInstance

num_spin_orbitals = 8
num_particles = (2,2)
qubit_converter = QubitConverter(mapper=JordanWignerMapper())
expectation = AerPauliExpectation()
backend = AerSimulator(method='statevector')
quantum_instance = QuantumInstance(backend=backend)

hf_state = HartreeFock(num_spin_orbitals=num_spin_orbitals, num_particles=num_particles, qubit_converter=qubit_converter)
ansatz = UCCSD(num_spin_orbitals=num_spin_orbitals, num_particles=num_particles, qubit_converter=qubit_converter, initial_state=hf_state)

#ansatz.num_qubits
#ansatz.parameters
#ansatz.num_parameters
ansatz = transpile(ansatz, backend=backend, optimization_level=3)

results in the following traceback and error message:

Traceback (most recent call last):
  File "/Users/joelbierman/Desktop/qiskit_bugtest/test_UCCSD_transpile.py", line 22, in <module>
    ansatz = transpile(ansatz, backend=backend, optimization_level=3)
  File "/Users/joelbierman/opt/anaconda3/envs/Qiskit-0360-pyscf2/lib/python3.9/site-packages/qiskit/compiler/transpiler.py", line 304, in transpile
    circuits = parallel_map(_transpile_circuit, list(zip(circuits, transpile_args)))
  File "/Users/joelbierman/opt/anaconda3/envs/Qiskit-0360-pyscf2/lib/python3.9/site-packages/qiskit/tools/parallel.py", line 129, in parallel_map
    return [task(values[0], *task_args, **task_kwargs)]
  File "/Users/joelbierman/opt/anaconda3/envs/Qiskit-0360-pyscf2/lib/python3.9/site-packages/qiskit/compiler/transpiler.py", line 389, in _transpile_circuit
    result = pass_manager.run(
  File "/Users/joelbierman/opt/anaconda3/envs/Qiskit-0360-pyscf2/lib/python3.9/site-packages/qiskit/transpiler/passmanager.py", line 222, in run
    return self._run_single_circuit(circuits, output_name, callback)
  File "/Users/joelbierman/opt/anaconda3/envs/Qiskit-0360-pyscf2/lib/python3.9/site-packages/qiskit/transpiler/passmanager.py", line 277, in _run_single_circuit
    result = running_passmanager.run(circuit, output_name=output_name, callback=callback)
  File "/Users/joelbierman/opt/anaconda3/envs/Qiskit-0360-pyscf2/lib/python3.9/site-packages/qiskit/transpiler/runningpassmanager.py", line 116, in run
    dag = circuit_to_dag(circuit)
  File "/Users/joelbierman/opt/anaconda3/envs/Qiskit-0360-pyscf2/lib/python3.9/site-packages/qiskit/converters/circuit_to_dag.py", line 62, in circuit_to_dag
    dagcircuit.apply_operation_back(instruction.copy(), qargs, cargs)
  File "/Users/joelbierman/opt/anaconda3/envs/Qiskit-0360-pyscf2/lib/python3.9/site-packages/qiskit/dagcircuit/dagcircuit.py", line 555, in apply_operation_back
    self._check_bits(qargs, self.output_map)
  File "/Users/joelbierman/opt/anaconda3/envs/Qiskit-0360-pyscf2/lib/python3.9/site-packages/qiskit/dagcircuit/dagcircuit.py", line 441, in _check_bits
    raise DAGCircuitError(f"(qu)bit {wire} not found in {amap}")
qiskit.dagcircuit.exceptions.DAGCircuitError: "(qu)bit Qubit(QuantumRegister(8, 'q'), 0) not found in OrderedDict()"

Un-commenting out any of the lines ansatz.num_qubits, ansatz.num_parameters, or ansatz.parameters allows the program to transpile the circuit without error.

What should happen?

Transpiling UCCSD should not require adding random lines that don't do anything except refer to some of its attributes.

Any suggestions?

No response

woodsp-ibm commented 2 years ago

UCCSD is a BlueprintCircuit based on the EvolvedOperator in Terra which means that the circuit can change and it does not need to be complete when constructed but can be setup/changed later. The 'random lines' yo added to read parameters do in fact cause it to do actions internally to allow it to pass back values, and it may need to build out aspects to do this. E.g. asking for num_qubits requires the EvolvedOperator to know that from the operators which it requires UCCSD to build etc. Anyway bottom line it seems that while everything could be built at the outset in your case, the operators, are not. But when EvolvedOperator has the operators it also sets up a qregs based on the number of qubits and without that the transpile is failing.

JoelHBierman commented 2 years ago

It does make sense to me that calling these attributes would have internal effects that change the UCCSD instance. By 'random lines' I mean that from a user perspective, it's not at all obvious that adding these lines would be necessary for utilizing functionality offered by other QuantumCircuit objects that are typically used for the same purposes. I essentially stumbled upon this workaround completely by accident. e.g. EfficientSU2, RealAmplitudes, ect. are also typically used as ansatzes for variational algorithms, but they do not behave this way. (Presumably because they are not EvolvedOperators.)

The things I am still unsure of are: 1. Whether this is a bug or the expected/intended behavior and 2. If it is a bug, whether or not there can be minimal changes made to the UCCSD code that makes using this functionality a bit more intuitive .

woodsp-ibm commented 2 years ago

It is not good overall behavior on the part of the objects, I agree. Since you have the object fully built by the constructor it ought to be completely valid at that point and not fail transpilation etc. Yes, I have a change, that I was looking at this morning, after I wrote the above, that improves things so it works as one might expect and your example code passes without any of the "random" lines.

JoelHBierman commented 2 years ago

Great, thank you! Once that merges I will close this issue.

woodsp-ibm commented 2 years ago

The way we normally do things is that the PR which fixes/implements an issue will auto-close the issue when the PR is merged. As you can see in the text above your comment ... linked a pull request 32 minutes ago that will close this issue. If you want to try the code - though you can see test cases like your example. that fully build and then transpile, and check things are fine yourself - you can install from source here once that PR is merged (or make the small change locally in your code - its the one liner _ = self.operators which calls that for the possible side-effect of building them out and as a by product knowing the number of qubits in the EvolvedOperator ansatz where they are set and the qregs is set also.