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
301 stars 204 forks source link

Fix issue1258 #1259

Closed grossardt closed 1 year ago

grossardt commented 1 year ago

Summary

Fixes issue #1258

For BosonicOp, FermionicOp, SpinOp, VibrationalOp, changed the line that takes care of the empty string coefficient (constant part of operator) in from_polynomial_tensor constructor to assign a scalar number rather than 0d array.

Details and comments

PolynomialTensor stores all coefficients as tensors, including the empty string contribution, which is consistent. The SparseLabelOps have a scalar number coefficient per label string. When constructed from PolynomialTensor, coefficients are assigned from the scalar elements of the corresponding tensor. However, the constant term (empty string) is dealt with separately, assigning the 0d tensor:

data[""] = tensor[key]

To convert this also to scalar, this is replaced by:

data[""] = tensor[key].sum()
CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

CLAassistant commented 1 year ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 6315372064


Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_nature/second_q/operators/bosonic_op.py 0 1 0.0%
qiskit_nature/second_q/operators/spin_op.py 0 1 0.0%
qiskit_nature/second_q/operators/vibrational_op.py 0 1 0.0%
<!-- Total: 1 4 25.0% -->
Files with Coverage Reduction New Missed Lines %
qiskit_nature/second_q/operators/tensor.py 1 85.46%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 6195281139: -0.01%
Covered Lines: 8706
Relevant Lines: 10038

💛 - Coveralls
grossardt commented 1 year ago

Sorry, I misinterpreted the CONTRIBUTING.md, assuming that fixes like this don't count as 'end user facing' and therefore don't need release notes. I added a short release note now, I hope this is appropriate in length and detail.

woodsp-ibm commented 1 year ago

Maybe this might logically be better to convert to scalar https://numpy.org/doc/stable/reference/generated/numpy.ndarray.item.html ? While sum works you did have to comment to explain what its really doing.

grossardt commented 1 year ago

@woodsp-ibm Thanks. I changed it accordingly. Tried to keep it compact without messing around with dtype and still get a scalar of the right type, but I simply didn't know of the existence of ndarray.item(), thats why .sum() seemed most fitting.