Skip to content
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

UCCSD Refactoring #66

Merged
merged 85 commits into from
Mar 25, 2021
Merged

Conversation

mrossinek
Copy link
Member

@mrossinek mrossinek commented Feb 26, 2021

Summary

Closes #57, closes #51, closes #55

Details and comments

For more details see the discussion there. I will update this description as we go.

This adds some drafts of the classes we plan to add during the
refactoring of UCCSD. More details can be found in
qiskit-community#57 (comment)
@Cryoris
Copy link
Contributor

Cryoris commented Feb 26, 2021

Can we put the circuits in qiskit_nature/circuit/library analogous to the other application stacks and Qiskit core?

@mrossinek
Copy link
Member Author

Can we put the circuits in qiskit_nature/circuit/library analogous to the other application stacks and Qiskit core?

All locations and namings are very temporary. I just wanted to get this out there. But yes, this is definitely something we can do!

manoelmarques and others added 4 commits February 26, 2021 17:02
This is an early attempt on what I think UCC may look like.
I am still unsure where to place the `ExcitationOpBuilder` in the code.
qiskit_nature/circuit/library/ansaetze/ucc.py Outdated Show resolved Hide resolved
qiskit_nature/circuit/library/ansaetze/ucc.py Outdated Show resolved Hide resolved
qiskit_nature/circuit/library/ansaetze/ucc.py Outdated Show resolved Hide resolved
qiskit_nature/circuit/library/ansaetze/ucc.py Outdated Show resolved Hide resolved
manoelmarques and others added 12 commits March 5, 2021 10:41
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Rather than duplicating the adaptive nature of an Ansatz through the
introduction an additional interface we can rely on an adaptive
algorithm to handle the list of excitations. Currently (AdaptVQE) does
this anyways and the `push` and `pop` methods were merely utilities.
Now, the BlueprintCircuit can simply have its operators replaced which
will automatically invalidate a cached circuit and will cause rebuilding
of all relevant information.
@mrossinek
Copy link
Member Author

Regarding mp2info I think it is straightforward to calculate from the excitation builder and the integrals. It should be kept attached to the UCCSD since the mp2 coeffs can be good approximations for initial guess of the cluster coefficients.

To clarify: I did not mean to outright remove it, merely to postpone it until a feature release in the future.
We discussed offline and agreed on doing exactly that.

@mrossinek mrossinek mentioned this pull request Mar 23, 2021
4 tasks
@pbark pbark changed the title WIP: UCCSD Refactoring UCCSD Refactoring Mar 24, 2021
@mrossinek mrossinek merged commit df053e0 into qiskit-community:master Mar 25, 2021
@mrossinek mrossinek deleted the uccsd-refactoring branch March 25, 2021 09:58
@mrossinek
Copy link
Member Author

@woodsp-ibm I know that you did not have time to review this yesterday but we need to continue with the algorithms where we rely on this work. Thus I merged it.
If you find something that needs to change, please comment here and tag me. Then I can address it in a follow-up PR 👍

mrossinek added a commit to mrossinek/qiskit-nature that referenced this pull request Mar 25, 2021
* Add drafts of classes for UCCSD refactoring

This adds some drafts of the classes we plan to add during the
refactoring of UCCSD. More details can be found in
qiskit-community#57 (comment)

* Fix mypy, spelling, lint

* add rough draft of EvolvedOpAnsatz

* Move EvolvedOpAnsatz to circuit.library.ansaetze

* Add rough draft of UCC

This is an early attempt on what I think UCC may look like.
I am still unsure where to place the `ExcitationOpBuilder` in the code.

* Some improvements based on code reviews

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Update ExcitationBuilder

* Move adaptive Ansätze to circuit.library submodule

* Add ideas for unittests

* Remove AdaptiveAnsatz interface

Rather than duplicating the adaptive nature of an Ansatz through the
introduction an additional interface we can rely on an adaptive
algorithm to handle the list of excitations. Currently (AdaptVQE) does
this anyways and the `push` and `pop` methods were merely utilities.
Now, the BlueprintCircuit can simply have its operators replaced which
will automatically invalidate a cached circuit and will cause rebuilding
of all relevant information.

* Fix excitation operators

* Assert qubit_op_converter during config checking

* Fix unittest for ExcitationBuilder

* Replace dummy QubitOpConverter class

* Fix linters

* Preserve excitation order

We must preserve the excitation order to enforce unique operators during
separate runs. By using lists rather than sets, this can be easily
achieved.

* Add arguments to filter excitations by spin type

* Move circuit/library tests into submodules

* Expose filter keyword arguments in UCC class

* Remove uppercase from spelling dict

* Fix error message

* Add UCC to circuit.library module imports

* Add UCCSD convenience subclass of UCC

* Minor update to UCC unittests

* Fix spell

* Rename "ansaetze" -> "ansatzes"

* Replace `pure_spin` with `max_spin_excitation`

The original intent of `pure_spin` was to add an easy configuration
option to specify the SUCC Ansatz. However, the original implementation
was backwards: `pure_spin` removed the alpha,beta double excitations but
in fact, we want to remove the alpha,alpha and beta,beta ones.

In the new implementation we have the optional integer argument called
`max_spin_excitation`. This can be used to set the
    maximum number of excitations within identical spin orbitals!

For example, setting `num_excitations=2` will compute all possible
double excitations. However, in combination with `max_spin_excitation=1`
this list will be filtered as to only include those where per spin, we
perform at most 1 excitation.

* Move build_excitation_ops outside of a class

* Various minor improvements

* Do not include permutations of excitations

* Replace UCCSD in VQEUCCSDFactory

* Fix spell

* Improve docstrings

* Fix spell

* Split excitation operator builder

Rather than generating the excitations and building the operators within
the same method, we split this into two parts for increased flexibility.

* Add the SUCCD convenience subclass

* Add the PUCCD convenience subclass

* Extract num_particles validation

* Fix mypy

* Add single excitation inclusion options

* Add some debug-level logging

* Make excitation genertor fermionic by naming

* Fix docstring

* WIP: vibrational mode excitation generator

* Fix spell

* Actually fix spell

* Update docstrings

* Add/cleanups unittests for excitation generators

* Improve UCCSD test

* Add proper tests for PUCCD and SUCCD

* Add more tests for plain UCC

* Add spell exceptions

* Make mypy happy

* Fix spell

* Fix unittest

* Fix operator caching

* Fix spell

* Remove MP2Info

We will refactor MP2Info in the future and add it as part of a feature
release.

* Add initial_state to UCC

* Fix after merge

* Add preferred_init_points

* add tests

* Fix lint

* evolved op tests

* fix spell

* Move initial_state property to EvolvedOpAnsatz

* Ensure PauliTrotterEvolution is used for UCC

* Move preferred_init_points property to EvolvedOpAnsatz

Co-authored-by: Manoel Marques <manoel.marques@ibm.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
mrossinek added a commit to mrossinek/qiskit-nature that referenced this pull request Mar 25, 2021
* Add drafts of classes for UCCSD refactoring

This adds some drafts of the classes we plan to add during the
refactoring of UCCSD. More details can be found in
qiskit-community#57 (comment)

* Fix mypy, spelling, lint

* add rough draft of EvolvedOpAnsatz

* Move EvolvedOpAnsatz to circuit.library.ansaetze

* Add rough draft of UCC

This is an early attempt on what I think UCC may look like.
I am still unsure where to place the `ExcitationOpBuilder` in the code.

* Some improvements based on code reviews

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Update ExcitationBuilder

* Move adaptive Ansätze to circuit.library submodule

* Add ideas for unittests

* Remove AdaptiveAnsatz interface

Rather than duplicating the adaptive nature of an Ansatz through the
introduction an additional interface we can rely on an adaptive
algorithm to handle the list of excitations. Currently (AdaptVQE) does
this anyways and the `push` and `pop` methods were merely utilities.
Now, the BlueprintCircuit can simply have its operators replaced which
will automatically invalidate a cached circuit and will cause rebuilding
of all relevant information.

* Fix excitation operators

* Assert qubit_op_converter during config checking

* Fix unittest for ExcitationBuilder

* Replace dummy QubitOpConverter class

* Fix linters

* Preserve excitation order

We must preserve the excitation order to enforce unique operators during
separate runs. By using lists rather than sets, this can be easily
achieved.

* Add arguments to filter excitations by spin type

* Move circuit/library tests into submodules

* Expose filter keyword arguments in UCC class

* Remove uppercase from spelling dict

* Fix error message

* Add UCC to circuit.library module imports

* Add UCCSD convenience subclass of UCC

* Minor update to UCC unittests

* Fix spell

* Rename "ansaetze" -> "ansatzes"

* Replace `pure_spin` with `max_spin_excitation`

The original intent of `pure_spin` was to add an easy configuration
option to specify the SUCC Ansatz. However, the original implementation
was backwards: `pure_spin` removed the alpha,beta double excitations but
in fact, we want to remove the alpha,alpha and beta,beta ones.

In the new implementation we have the optional integer argument called
`max_spin_excitation`. This can be used to set the
    maximum number of excitations within identical spin orbitals!

For example, setting `num_excitations=2` will compute all possible
double excitations. However, in combination with `max_spin_excitation=1`
this list will be filtered as to only include those where per spin, we
perform at most 1 excitation.

* Move build_excitation_ops outside of a class

* Various minor improvements

* Do not include permutations of excitations

* Replace UCCSD in VQEUCCSDFactory

* Fix spell

* Improve docstrings

* Fix spell

* Split excitation operator builder

Rather than generating the excitations and building the operators within
the same method, we split this into two parts for increased flexibility.

* Add the SUCCD convenience subclass

* Add the PUCCD convenience subclass

* Extract num_particles validation

* Fix mypy

* Add single excitation inclusion options

* Add some debug-level logging

* Make excitation genertor fermionic by naming

* Fix docstring

* WIP: vibrational mode excitation generator

* Fix spell

* Actually fix spell

* Update docstrings

* Add/cleanups unittests for excitation generators

* Improve UCCSD test

* Add proper tests for PUCCD and SUCCD

* Add more tests for plain UCC

* Add spell exceptions

* Make mypy happy

* Fix spell

* Fix unittest

* Fix operator caching

* Fix spell

* Remove MP2Info

We will refactor MP2Info in the future and add it as part of a feature
release.

* Add initial_state to UCC

* Fix after merge

* Add preferred_init_points

* add tests

* Fix lint

* evolved op tests

* fix spell

* Move initial_state property to EvolvedOpAnsatz

* Ensure PauliTrotterEvolution is used for UCC

* Move preferred_init_points property to EvolvedOpAnsatz

Co-authored-by: Manoel Marques <manoel.marques@ibm.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Anthony-Gandon pushed a commit to Anthony-Gandon/qiskit-nature that referenced this pull request May 25, 2023
* Add drafts of classes for UCCSD refactoring

This adds some drafts of the classes we plan to add during the
refactoring of UCCSD. More details can be found in
qiskit-community#57 (comment)

* Fix mypy, spelling, lint

* add rough draft of EvolvedOpAnsatz

* Move EvolvedOpAnsatz to circuit.library.ansaetze

* Add rough draft of UCC

This is an early attempt on what I think UCC may look like.
I am still unsure where to place the `ExcitationOpBuilder` in the code.

* Some improvements based on code reviews

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Update ExcitationBuilder

* Move adaptive Ansätze to circuit.library submodule

* Add ideas for unittests

* Remove AdaptiveAnsatz interface

Rather than duplicating the adaptive nature of an Ansatz through the
introduction an additional interface we can rely on an adaptive
algorithm to handle the list of excitations. Currently (AdaptVQE) does
this anyways and the `push` and `pop` methods were merely utilities.
Now, the BlueprintCircuit can simply have its operators replaced which
will automatically invalidate a cached circuit and will cause rebuilding
of all relevant information.

* Fix excitation operators

* Assert qubit_op_converter during config checking

* Fix unittest for ExcitationBuilder

* Replace dummy QubitOpConverter class

* Fix linters

* Preserve excitation order

We must preserve the excitation order to enforce unique operators during
separate runs. By using lists rather than sets, this can be easily
achieved.

* Add arguments to filter excitations by spin type

* Move circuit/library tests into submodules

* Expose filter keyword arguments in UCC class

* Remove uppercase from spelling dict

* Fix error message

* Add UCC to circuit.library module imports

* Add UCCSD convenience subclass of UCC

* Minor update to UCC unittests

* Fix spell

* Rename "ansaetze" -> "ansatzes"

* Replace `pure_spin` with `max_spin_excitation`

The original intent of `pure_spin` was to add an easy configuration
option to specify the SUCC Ansatz. However, the original implementation
was backwards: `pure_spin` removed the alpha,beta double excitations but
in fact, we want to remove the alpha,alpha and beta,beta ones.

In the new implementation we have the optional integer argument called
`max_spin_excitation`. This can be used to set the
    maximum number of excitations within identical spin orbitals!

For example, setting `num_excitations=2` will compute all possible
double excitations. However, in combination with `max_spin_excitation=1`
this list will be filtered as to only include those where per spin, we
perform at most 1 excitation.

* Move build_excitation_ops outside of a class

* Various minor improvements

* Do not include permutations of excitations

* Replace UCCSD in VQEUCCSDFactory

* Fix spell

* Improve docstrings

* Fix spell

* Split excitation operator builder

Rather than generating the excitations and building the operators within
the same method, we split this into two parts for increased flexibility.

* Add the SUCCD convenience subclass

* Add the PUCCD convenience subclass

* Extract num_particles validation

* Fix mypy

* Add single excitation inclusion options

* Add some debug-level logging

* Make excitation genertor fermionic by naming

* Fix docstring

* WIP: vibrational mode excitation generator

* Fix spell

* Actually fix spell

* Update docstrings

* Add/cleanups unittests for excitation generators

* Improve UCCSD test

* Add proper tests for PUCCD and SUCCD

* Add more tests for plain UCC

* Add spell exceptions

* Make mypy happy

* Fix spell

* Fix unittest

* Fix operator caching

* Fix spell

* Remove MP2Info

We will refactor MP2Info in the future and add it as part of a feature
release.

* Add initial_state to UCC

* Fix after merge

* Add preferred_init_points

* add tests

* Fix lint

* evolved op tests

* fix spell

* Move initial_state property to EvolvedOpAnsatz

* Ensure PauliTrotterEvolution is used for UCC

* Move preferred_init_points property to EvolvedOpAnsatz

Co-authored-by: Manoel Marques <manoel.marques@ibm.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor UCCSD Hamiltonian Variational Form (TASP) same_spin_doubles works incorrectly in UCCSD
5 participants