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

Modular Driver Result #263

Merged
merged 76 commits into from
Jul 30, 2021
Merged

Conversation

mrossinek
Copy link
Member

@mrossinek mrossinek commented Jul 5, 2021

Summary

This PR implements #243 and is the second part of #148.

Details and comments

Tasks which will be left towards the third (and final) part of #148: [EDIT: #264]

  • direct integration of DriverResult with drivers (i.e. do not return QMolecule as intermediate storage container)
  • deprecation of BaseProblem.molecule_data* properties
  • implementation of HDF5 interface (storing and loading similar to QMolecule)
  • release note
  • tutorial (if time permits)

mrossinek added 20 commits July 2, 2021 09:30
This is an early prototype of the ASTransfomer based on the Property objects.
A few changes were necessary:
- assembly of all ElectronicBases on the IntegralProperty objects (rather than different bases
  instances on the ElectronicDriverResult)
- introduction of the `reduce_system_size` method on the SecondQuantizedProperty object (this makes
  the ASTransformer a first class citizen of the Property stack but provides the most straight
  forward method of ensuring pluggability and consistency insurance)
@mrossinek
Copy link
Member Author

Alright I addressed everything that was possible for now. Some final discussion on naming and around the driver results themselves can still happen as part of the third PR. Let's try to get this merged by Friday 👍

@mrossinek mrossinek requested review from woodsp-ibm and pbark July 28, 2021 16:05
@mrossinek mrossinek requested a review from woodsp-ibm July 29, 2021 12:57
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.

No further comments, it seems fine.

@mrossinek mrossinek dismissed dlasecki’s stale review July 30, 2021 07:05

All requested changes have been implemented

Copy link
Contributor

@pbark pbark left a comment

Choose a reason for hiding this comment

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

This now LGTM! Thanks a lot for the work @mrossinek and for the thorough reviews @dlasecki and @woodsp-ibm !

@mergify mergify bot merged commit 3a5a029 into qiskit-community:main Jul 30, 2021
@mrossinek mrossinek deleted the modular_driver_result branch July 30, 2021 08:47
@mrossinek mrossinek mentioned this pull request Jul 30, 2021
@mrossinek mrossinek mentioned this pull request Aug 4, 2021
9 tasks
Anthony-Gandon pushed a commit to Anthony-Gandon/qiskit-nature that referenced this pull request May 25, 2023
* WIP: add initial (dumb) ElectronicDriverResult

* Extract IntegralProperty

* WIP: ActiveSpaceTransformer migration

This is an early prototype of the ASTransfomer based on the Property objects.
A few changes were necessary:
- assembly of all ElectronicBases on the IntegralProperty objects (rather than different bases
  instances on the ElectronicDriverResult)
- introduction of the `reduce_system_size` method on the SecondQuantizedProperty object (this makes
  the ASTransformer a first class citizen of the Property stack but provides the most straight
  forward method of ensuring pluggability and consistency insurance)

* More WIP stuff

* Simplify ElectronicIntegral juggling

* Pass list of active orbital indices to reduce_system_size method

* Improve property handling

* Improve electronic integral access

* Fix TotalDipoleMoment as Iterable Property

* Extract CompositeProperty

* Improve ASTrafo readability

* Run black

* Docs/repr updates

* Fix Property tests

* Add compose docs

* Integrate ElectronicDriverResult into ElectronicStructureProblem

* Fix linters

* Fix mypy

* Extract DriverResult

* Fix spell

* Fix lint

* Straight forward extraction of interpret methods

* Migrate FreezeCoreTransformer

* Fix tests

* More fixes

* Fix for pylint<2.9

* Protect against None-cases

* Fix false positive under Python 3.6

* More None-safety

* Run black

* Fix mypy

* Properly deprecate build_ferm_op_from_ints

* Deprecate old Transformer interface

* Unittest + docs for electronic.integrals

* Unittests for CompositeProperty

* More unittests

* More docs

* Remove unused import

* Commit review suggestion

Co-authored-by: Dariusz Lasecki <dal@zurich.ibm.com>

* Join string representation only once

* Extend __repr__ and flat internal integral storage to vibrational properties

* Add error messages and warnings

* Fix args documentation

* Fix linters

* Apply suggestions from code review

Co-authored-by: Panagiotis Barkoutsos <pbarkoutsos@gmail.com>

* More suggestions from code review

* Define __str__ instead of __repr__

* Fix lint

* Add driver metadata

* Rename CompositeProperty -> GroupedProperty

* Move Molecule.count_orbitals to FreezeCoreTransformer.count_core_orbitals

* Document dual purpose of ParticleNumber property

* Clarify distinction of electronic, nuclear and total energy/dipole components

* Add additional (purely informational) data to ElectronicEnergy

* Fix ParticleNumber.interpret method

* Fix transformer deprecation

* Review suggestions

* Improved electronic/vibrational property subtyping

* Extend Transformer interface to GroupedProperty

* Move reduce_system_size logic directly into Transformer

* Rename matrix_operator() -> integral_operator()

* Rename Electronic/Vibrational*DriverResult -> Electronic/Vibrational*StructureDriverResult

* Fix spelling

* Left over fixes

* Fix astroid constraint

* Fix docstring

* Remove (so far) unused ElectronicEnergy.fock attr

* Fix lint

Co-authored-by: Dariusz Lasecki <dal@zurich.ibm.com>
Co-authored-by: Panagiotis Barkoutsos <pbarkoutsos@gmail.com>
Co-authored-by: Manoel Marques <Manoel.Marques@ibm.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.

5 participants