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

refactor: move code to new structure #746

Merged
merged 4 commits into from
Jul 18, 2022
Merged

refactor: move code to new structure #746

merged 4 commits into from
Jul 18, 2022

Conversation

mrossinek
Copy link
Member

@mrossinek mrossinek commented Jul 15, 2022

Summary

This is the follow-up to #722

Details and comments

This PR uses the approach describes in https://stackoverflow.com/a/44036771 to "copy" files while preserving the blame history in both locations.

⚠️ This PR must be merged withOUT squashing all commits (I may however still be able to clean up some of the commits (e.g. the CI changes and main merges)

Closes #701 #702 #703 #704 #705 #706 #707 #708 #709 #710 #711

@mrossinek

This comment was marked as resolved.

@mrossinek

This comment was marked as resolved.

@mrossinek mrossinek marked this pull request as draft July 15, 2022 16:45
ElePT and others added 3 commits July 18, 2022 06:41
* Move operators

* Remove ops, move mappers

* Refactor fermionic mapper imports

* Remove old mappers

* Move qubit converter

* Refactor qubit mapper imports

* Refactor qubit converter imports

* Remove ListorDict

* Remove old converters

* Move operator tests

* Remove old operator tests

* Refactor operator test imports

* Move mappers tests

* Remove old mappers tests

* Refactor mapper test imports

* Update operators inits

* Update operator tests inits

* Fix black

* Fix lint

* Fix copyright

* Delete __init__.py

Remove extra init

* Fix imports, lint

* Fix copyright

* Migrate converters tests

* Fix imports of operators

* Fix imports of mappers

* Replace qubit converter imports

* Restore releasenotes

* Restore releasenotes

* Restore releasenotes

* Remove extra init

* Add QubitConverter

* Move properties to operator factories

* Move problems

* Move results

* Move properties, results and problems tests

* Change problem imports

* Change properties imports

* Change results imports

* Change properties imports

* Fix property imports

* Refactor problem inits

* Fix black

* Fix results imports

* Fix qubit mapper import

* Fix copyright

* Fix various imports

* undo hdf5 changes

* Replace second_quantization with second_q

* Remove init (again????)

* Restore ListOrDict

* Undo remove LisOrDict

* Fix ListOrDict

* Fix ListOrDict

* Rename test second_quantization, various lint fixes

* Fix lint

* Fix black/copyright

* Restore converters init

* Moved transformers

* Restore converters init

* Move circuit

* Move algorithms

* Move algorithms and circuit unit tests

* Replace algorithms imports

* Replace circuit imports

* Remove init againnnnnn

* fix transformers imports +

* Fix transformers imports

* Remove 'leftovers'

* Migrate drivers

* Fix driver imports

* Fix all local lint errors

* Fix black and copyright

* Fix import

* Fix driver import

* Move ListOrdict

* refactor: update HDF5 files

This updates the contents of the HDF5 files used during unittesting.
Since this is a temporary migration of all the existing properties into
a new module location, this means that the module import paths stored
inside of the HDF5 files are being updated by this PR. Nothing else
changes functionally.

* fix cyclic imports

* Remove drivers/sescond_quantization submodule

* fix drivers imports

* fix cyclic imports

* Fix str mistake

* Fix black

* Fix cyclic import driver result

* Update pylint minimum version

* Inline imports molecule

* Modify resource imports

* Turn requirement into constraint

* Update constraint

* fix driver imports

* Fix black

* Fix transformers test

* Fix typo

* Fix black

* 2nd attempt fix cyclic import drivers

* Fix black

* Remove sampling tutorial

* Remove protein folding tutorial

* Fix path

* Remove pes sampler import

* Fix black

* Fix tutorial

* Find drivers

* Fix tutorials

* fix drivers tests

* fix drivers tests

* Modify tutorial explanations (not finished)

* Move mappers and converter

* Move mappers test

* Fix mapper imports

* Fix black

* Flatten operators

* Migrate transformers

* Migrate problems

* Refactor operator_factories into properties (again)

* Rescue mapper base classes

* Rescue hopping ops

* Fix import

* fix: HDF5 binary files and file locations

* fix: more test resource paths

* fix: final resource path fixes

* docs: auto-generate docs/apidocs with Sphinx

* Fix tutorial import

* Update inits

* Fix imports

* Fix import 2

* Remove pes samplers

* Fix import

* Fix sphinx

* Fix docs

* Fix black

* Fix docs

* Rubber duck

* fix docs

* Update docs/getting_started.rst

"Remove link"

Co-authored-by: Max Rossmannek <max.rossmannek@uzh.ch>

* Update qiskit_nature/second_q/properties/vibrational_energy.py

"Apply fix"

Co-authored-by: Max Rossmannek <max.rossmannek@uzh.ch>

* Update qiskit_nature/second_q/properties/vibrational_energy.py

Co-authored-by: Max Rossmannek <max.rossmannek@uzh.ch>

* Update qiskit_nature/second_q/properties/vibrational_energy.py

Co-authored-by: Max Rossmannek <max.rossmannek@uzh.ch>

* Update test/nature_test_case.py

Co-authored-by: Max Rossmannek <max.rossmannek@uzh.ch>

* Update test/nature_test_case.py

Co-authored-by: Max Rossmannek <max.rossmannek@uzh.ch>

* Update qiskit_nature/second_q/properties/bases/__init__.py

Co-authored-by: Max Rossmannek <max.rossmannek@uzh.ch>

* Update qiskit_nature/second_q/properties/property.py

Co-authored-by: Max Rossmannek <max.rossmannek@uzh.ch>

* Apply review comments

* Fix path

* Apply comments

* Move quadratic hamiltonian + random fixes

* Fix lint

* Fix mypy

* fix import

* Add hamiltonians to docs

* Fix black

* Add hamiltonians to toctree

* Apply suggestions from code review

* Apply review comment

* Apply review comments

* Fix black

* Fix sneaky mappers

Co-authored-by: Max Rossmannek <oss@zurich.ibm.com>
Co-authored-by: Manoel Marques <Manoel.Marques@ibm.com>
Co-authored-by: Max Rossmannek <max.rossmannek@uzh.ch>
This is based on the suggestion [1], which will allow us to preserve the
git blaming history in both, the old and new, code locations.

This will be reverted after the move to the new code location has been
merged with this commit.

[1]: https://stackoverflow.com/a/44036771
@mrossinek
Copy link
Member Author

@mrossinek mrossinek marked this pull request as ready for review July 18, 2022 06:15
@mrossinek
Copy link
Member Author

I also marked this PR to be closing all of the code relocation issues. I will open separate issues for the planned refactorings within the modules in the near future 👍

@ElePT
Copy link
Contributor

ElePT commented Jul 18, 2022

Yay! Git blame works for both the old and the new modules!

@coveralls
Copy link

coveralls commented Jul 18, 2022

Pull Request Test Coverage Report for Build 2691663016

  • 7854 of 9396 (83.59%) changed or added relevant lines in 150 files are covered.
  • 7 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-0.5%) to 84.199%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_nature/list_or_dict.py 17 18 94.44%
qiskit_nature/second_q/algorithms/excited_states_solvers/eigensolver_factories/eigensolver_factory.py 7 8 87.5%
qiskit_nature/second_q/algorithms/excited_states_solvers/excited_states_solver.py 15 16 93.75%
qiskit_nature/second_q/circuit/library/ansatzes/puccd.py 39 40 97.5%
qiskit_nature/second_q/circuit/library/ansatzes/succd.py 42 43 97.67%
qiskit_nature/second_q/circuit/library/initial_states/hartree_fock.py 37 38 97.37%
qiskit_nature/second_q/circuit/library/initial_states/vscf.py 31 32 96.88%
qiskit_nature/second_q/drivers/base_driver.py 6 7 85.71%
qiskit_nature/second_q/drivers/electronic_structure_driver.py 15 16 93.75%
qiskit_nature/second_q/drivers/gaussiand/gaussian_log_result.py 169 170 99.41%
Files with Coverage Reduction New Missed Lines %
qiskit_nature/properties/second_quantization/electronic/angular_momentum.py 1 88.57%
qiskit_nature/properties/second_quantization/electronic/dipole_moment.py 1 91.79%
qiskit_nature/properties/second_quantization/electronic/integrals/integral_property.py 1 90.36%
qiskit_nature/properties/second_quantization/electronic/magnetization.py 1 86.27%
qiskit_nature/properties/second_quantization/electronic/particle_number.py 1 91.23%
qiskit_nature/algorithms/excited_states_solvers/qeom.py 2 82.61%
Totals Coverage Status
Change from base Build 2677814739: -0.5%
Covered Lines: 17760
Relevant Lines: 21093

💛 - Coveralls

@ElePT
Copy link
Contributor

ElePT commented Jul 18, 2022

One small comment, the tutorials I removed (5,6 and 9) are still removed in this PR. It doesn't make sense to migrate them, but shouldn't we keep the old version of these together with the old version of the repo until we fully deprecate it? I also took a quick look at the docs and they seem ok to my untrained eye.

@mrossinek
Copy link
Member Author

One small comment, the tutorials I removed (5,6 and 9) are still removed in this PR. It doesn't make sense to migrate them, but shouldn't we keep the old version of these together with the old version of the repo until we fully deprecate it?

Hm that's a good point actually. For the docs I decided (based on a discussion with @manoelmarques and previous input from @woodsp-ibm) that only the new code is documented in the API (i.e. the old code no longer has API documentation). Nonetheless, I agree that it makes sense to keep the legacy tutorials in place. I will re-add those in an upcoming commit 👍

I also took a quick look at the docs and they seem ok to my untrained eye.

Thanks 👍

@mrossinek
Copy link
Member Author

For reviewing purposes it might be easiest, if you do a commit-by-commit review. The 4 commits do the following:

  1. merges Migrate modules to new location #722 (the relocation of the old code into the new structure, updating all imports, etc.)
  2. "only" moves all of the old code to some "dummy" locations and upgrades the copyright in the process.
  3. merges the previous two commits into one another (because these were parallel and not linear, since they moved the same files into separate locations). This merge commit changed 8 __init__ files mostly related to copyright.
  4. "reverts" the second commit, moving the old code back from the "dummy" locations into its original place. In doing so, some minor updates are included to ensure the CI works

@mrossinek mrossinek changed the title [refactor] move code to new structure refactor: move code to new structure Jul 18, 2022
@mrossinek
Copy link
Member Author

After an offline discussion, we decided to remove the tutorials for the code that is also no longer part of the public API docs. The code in question is still being unittested and the tutorials still exist on stable where they are being run, too 👍

@manoelmarques manoelmarques merged commit b0be0fb into main Jul 18, 2022
This was referenced Jul 19, 2022
@manoelmarques manoelmarques deleted the refactor branch August 2, 2022 13:07
Anthony-Gandon pushed a commit to Anthony-Gandon/qiskit-nature that referenced this pull request May 25, 2023
refactor: move code to new structure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants