Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

Replace the InitialState components by circuits #1374

Merged
merged 28 commits into from
Oct 30, 2020

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Oct 20, 2020

Summary

Convert HartreeFock and VSCF to circuits and deprecated the other initial state components. This fits the overall picture of using circuits as algorithm building blocks instead of specialized Aqua types.

Details and comments

Most of the functionality of the InitialState components is already available in the QuantumCircuit (essentially by using the initialize method). Therefore, these components are no longer necessary. The specialized initial states have been converted to QuantumCircuit types.
The compatibility of all algorithms/components has been updated to support initial states given as circuits.

The replacement of HartreeFock and VSCF is done in-place (i.e. they implement both QuantumCircuit and InitialState) until we decide where circuits in the applications module should be kept.

@Cryoris Cryoris added this to the 0.9 milestone Oct 20, 2020
@Cryoris Cryoris added Changelog: Deprecation Include in Deprecated section of changelog Changelog: New Feature Include in the Added section of the changelog labels Oct 20, 2020
stefan-woerner
stefan-woerner previously approved these changes Oct 23, 2020
Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since InitialStates are now circuits wasn't the intent to reflect the circuit library and have a qiskit.chemistry.circuit.library where presumably we would have an initial_states folder in there containing these. Leaving just stubs where they are now, the stub extending the now moved one so it can be still be used from where it was, but which emit deprecation messages to change to use them from the new location.

qiskit/chemistry/components/initial_states/hartree_fock.py Outdated Show resolved Hide resolved
return np.zeros(self._num_parameters, dtype=np.float)
else:
return None
return np.zeros(self._num_parameters, dtype=np.float)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this basically ends up suggesting to VQE that all zeros is the right start point no matter what initial state is supplied (when one is supplied). I guess this should be ok since for now this is normally used with HF init state. It was anticipated that for some other starting point like MP2 we would be indicating other values though clearly to date we have not had code that does this.

Custom(state_vector) | qc = QuantumCircuit(n)
| qc.initialize(state_vector, qc.qubits) # normalize vector!
Custom('random') | use the initialize instruction with random amplitudes
VarFormBased | use the circuit of the variational form directly
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need to provide some guidance somewhere on this due to the parameter sorting that is done by VQE? As long as you get the optimal dictionary I guess one is good - get the optimal params and you need to take care over sorting. I'm thinking of the use case where we take the var form that had been evolved somewhat by VQE and use that as the starting state for QPE - i.e. the test_vqe2iqpe that I see has been updated to do the above. Maybe we can show this in some tutorial as I am not sure there is a good place in the docs unless we did something in VQE

test/chemistry/test_uccsd_hartree_fock.py Outdated Show resolved Hide resolved
- move the initial state circuit to chemistry.circuit.library
- globally replace num_particles by a tuple instead of list of integers
@Cryoris
Copy link
Contributor Author

Cryoris commented Oct 27, 2020

I moved the circuits to qiskit.chemistry.circuit.library in 608aade 👍

Comment on lines -231 to +234
var_form = TwoLocal(num_qubits, ['ry', 'rz'], 'cz', initial_state=init_state)
var_form = TwoLocal(num_qubits, ['ry', 'rz'], 'cz')

# add the initial state
var_form.compose(init_state, front=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is a required change in that initial_state will not accommodate a QuantumCircuit? If it is now done this way it seems the var form will not have visibility to the initial state as it had when it was passed directly and it would prepend it itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had a workaround in place where the NLocal circuits expected a InitialState. I'll add support for passing a QuantumCircuit and deprecate the InitialStates once this is merged. We can update the readme sample to use the kwarg again.

two_qubit_reduction=self._two_qubit_reduction,
num_particles=self._molecule_info[self.INFO_NUM_PARTICLES])
z2_symmetries = Hamiltonian._pick_sector(z2_symmetries, hf_state.bitstr)
from ..circuit.library.initial_states.hartree_fock import _build_bitstr
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me this does not look so good running off a private member. Maybe the better thing to do is to have some util somewhere that can build the bitstr so that the HF state or anything else like this can use it. If nothing it should be at least a public method of HFState if its preferable to keep it there for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about these two options too, but the problem with these are, that

  • to extract the bitstring from the circuit we'd have to simulate the circuit and retrieve the bitstring, which is exponentially expensive
  • having the property for the bitstring on it is a bit weird because initial states don't generally have to have a bitstring
  • also we're creating a circuit but only care about the bitstring it is built from, this seems like an inverted workflow

How about changing this from a private function to a public method? E.g. hartree_fock_bitstring?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A public method seems fine - the main suggestion was some sort of utility method/module somewhere outside of the circuit that the circuit could use. We have no util folder at present, hence other things, like MP2Info are in the root. Adding another file there for one method seems too much. Hence having it there with the circuit as a public method seems ok for now. hartree_fock_bitstring or hf_bitstring seems fine to me.

@Cryoris Cryoris merged commit d88d0ad into qiskit-community:master Oct 30, 2020
@Cryoris Cryoris deleted the initial-state branch October 30, 2020 07:29
mtreinish pushed a commit to mtreinish/qiskit-core that referenced this pull request Nov 20, 2020
…kit-aqua#1374)

* make chemistry initial states circuits

* put VSCF circuit in new file

* update several tests

* fix type change to tuple

* merge hf initial state and circuit

* merge initstate+circuit

* update EOH

* update hhl and qaia

* update chc and uvcc

* deprecate InitialState components

* update varformbased test

* fix running tests & mypy

* add reno

* fix iqpe test

* fix variable renaming from merge

* move to circuit.lib and use tuple

- move the initial state circuit to chemistry.circuit.library
- globally replace num_particles by a tuple instead of list of integers

* make _build_bitstr public functions
manoelmarques pushed a commit to manoelmarques/qiskit-terra that referenced this pull request Dec 2, 2020
…kit-aqua#1374)

* make chemistry initial states circuits

* put VSCF circuit in new file

* update several tests

* fix type change to tuple

* merge hf initial state and circuit

* merge initstate+circuit

* update EOH

* update hhl and qaia

* update chc and uvcc

* deprecate InitialState components

* update varformbased test

* fix running tests & mypy

* add reno

* fix iqpe test

* fix variable renaming from merge

* move to circuit.lib and use tuple

- move the initial state circuit to chemistry.circuit.library
- globally replace num_particles by a tuple instead of list of integers

* make _build_bitstr public functions
manoelmarques pushed a commit to manoelmarques/qiskit-terra that referenced this pull request Dec 7, 2020
…kit-aqua#1374)

* make chemistry initial states circuits

* put VSCF circuit in new file

* update several tests

* fix type change to tuple

* merge hf initial state and circuit

* merge initstate+circuit

* update EOH

* update hhl and qaia

* update chc and uvcc

* deprecate InitialState components

* update varformbased test

* fix running tests & mypy

* add reno

* fix iqpe test

* fix variable renaming from merge

* move to circuit.lib and use tuple

- move the initial state circuit to chemistry.circuit.library
- globally replace num_particles by a tuple instead of list of integers

* make _build_bitstr public functions
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog: Deprecation Include in Deprecated section of changelog Changelog: New Feature Include in the Added section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants