Handle orbital overlap in spin operators

qiskit-community / qiskit-nature

Qiskit Nature is an open-source, quantum computing, framework for solving quantum mechanical natural science problems.

https://qiskit-community.github.io/qiskit-nature/

Apache License 2.0

290 stars 197 forks source link

Handle orbital overlap in spin operators #1275

Closed mrossinek closed 8 months ago

mrossinek commented 8 months ago

Summary

Fixes #1273. For more details, please refer to that issue.

Details and comments

[x] implements an actual fix for the underlying problem
[x] unittests the above
[x] use the above fix properly, directly within the drivers
- ~This is potentially problematic right now, because the QCSchema does not have a field to specify the overlap. I will need to investigate a good solution for this.~
- I settled on adding an additional attribute to the QCSchema which stores the AO overlap. This seems like the best solution at this time.
[x] ensure the more complex AngularMomentum is handled properly by any Transformer class
[x] add a release note

coveralls commented 8 months ago

Pull Request Test Coverage Report for Build 6866355451

79 of 86 (91.86%) changed or added relevant lines in 10 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+0.008%) to 86.768%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit_nature/second_q/transformers/active_space_transformer.py	11	12	91.67%
qiskit_nature/second_q/transformers/basis_transformer.py	10	12	83.33%
qiskit_nature/second_q/formats/qcschema_translator.py	22	26	84.62%
<!--	Total:	79	86	91.86%	-->

Files with Coverage Reduction	New Missed Lines	%
qiskit_nature/second_q/drivers/psi4d/psi4driver.py	1	85.12%
<!--	Total:	1		-->

Totals
Change from base Build 6860424299:	0.008%
Covered Lines:	8767
Relevant Lines:	10104

💛 - Coveralls

mrossinek commented 8 months ago

Do we really want to backport this? It is a significant bug, but this also changes the API of e.g. the AngularMomentum..

woodsp-ibm commented 8 months ago

The change is backward compatible though with any existing code is it not - the new parameter is optional and it will still do what it did before no? But technically it is an API change, though needed to sort the bug.