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.

Sign up for GitHub

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

Jump to bottom

Replace PauliSumOp with SparsePauliOp #1050

Merged
merged 4 commits into from
Feb 22, 2023
Merged

Replace PauliSumOp with SparsePauliOp #1050

merged 4 commits into from
Feb 22, 2023

Conversation

manoelmarques
Copy link
Contributor

Summary

Fixes #1020

Details and comments

@manoelmarques manoelmarques self-assigned this Feb 3, 2023
@mrossinek
Copy link
Member

It will be easiest to deal with this after #1044 is merged

@mrossinek mrossinek added the on hold Can not fix yet label Feb 3, 2023
@mrossinek mrossinek removed the on hold Can not fix yet label Feb 9, 2023
@mrossinek mrossinek force-pushed the pauli branch 5 times, most recently from a2233b9 to 5df155f Compare February 9, 2023 13:39
@coveralls
Copy link

coveralls commented Feb 9, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 4244341662

  • 148 of 163 (90.8%) changed or added relevant lines in 24 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 85.948%
Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_nature/deprecation.py 15 16 93.75%
qiskit_nature/second_q/algorithms/excited_states_solvers/qeom.py 3 4 75.0%
qiskit_nature/second_q/mappers/tapered_qubit_mapper.py 13 14 92.86%
qiskit_nature/second_q/algorithms/excited_states_solvers/excited_states_eigensolver.py 2 4 50.0%
qiskit_nature/settings.py 13 17 76.47%
qiskit_nature/second_q/algorithms/excited_states_solvers/qeom_electronic_ops_builder.py 5 11 45.45%
Totals Coverage Status
Change from base Build 4244333195: 0.03%
Covered Lines: 18049
Relevant Lines: 21000
💛 - Coveralls

@mrossinek mrossinek marked this pull request as ready for review February 9, 2023 15:22
mrossinek
mrossinek reviewed Feb 9, 2023
View reviewed changes
Copy link
Member

@mrossinek mrossinek left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Some initial comments (I need to do a more thorough review later).
Also some observations:

  • we need to add a release note
  • we should check for any more usage of opflow in the unittests and replace it wherever possible

qiskit_nature/settings.py Show resolved Hide resolved
qiskit_nature/second_q/mappers/interleaved_qubit_mapper.py Outdated Show resolved Hide resolved
qiskit_nature/second_q/algorithms/excited_states_solvers/excited_states_eigensolver.py Show resolved Hide resolved
@manoelmarques manoelmarques force-pushed the pauli branch 3 times, most recently from 0486a3b to 4f3ccfc Compare February 13, 2023 18:09
@mrossinek
Copy link
Member

I have applied the following updates to this PR:

  • removed the QubitOperator alias in favor of being explicit everywhere (in doing so I also removed the BaseOperator type hint which was inconsistently used and thus not really supported (and especially not tested; does this warrant mention in the release note?)
  • I updated type hints to the "future-style" in files touched by this PR
  • all usage of PauliSumOp is now explicitly findable by grepping for PauliSumOp. It is merely used for type hints and otherwise all "real logic" uses SparsePauliOp now
  • deprecation messages for PauliSumOp usage are in place thanks to @manoelmarques 👍

@mrossinek mrossinek requested a review from woodsp-ibm February 22, 2023 10:02
@mergify mergify bot merged commit ae78e2c into qiskit-community:main Feb 22, 2023
@mrossinek mrossinek mentioned this pull request Feb 22, 2023
Closed
Anthony-Gandon pushed a commit to Anthony-Gandon/qiskit-nature that referenced this pull request May 25, 2023
@manoelmarques @mrossinek
@woodsp-ibm @mergify
Replace PauliSumOp with SparsePauliOp (qiskit-community#1050)
bed7146 
* Replace PauliSumOp with SparsePauliOp

* Update releasenotes/notes/paulisumop-replace-d1ae54d1854b301b.yaml

Co-authored-by: Steve Wood <[email protected]>

* Adds warn_deprecated_type utility

---------

Co-authored-by: Max Rossmannek <[email protected]>
Co-authored-by: Steve Wood <[email protected]>
Co-authored-by: Max Rossmannek <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrossinek mrossinek mrossinek left review comments

@woodsp-ibm woodsp-ibm woodsp-ibm approved these changes

@stefan-woerner stefan-woerner Awaiting requested review from stefan-woerner

Assignees

@manoelmarques manoelmarques

Labels
automerge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace PauliSumOp with SparsePauliOp
4 participants
@manoelmarques @mrossinek @coveralls @woodsp-ibm