Skip to content

Implement the Tensor class#1033

Merged
mergify[bot] merged 8 commits into
qiskit-community:mainfrom
mrossinek:tensor
Feb 10, 2023
Merged

Implement the Tensor class#1033
mergify[bot] merged 8 commits into
qiskit-community:mainfrom
mrossinek:tensor

Conversation

@mrossinek
Copy link
Copy Markdown
Member

@mrossinek mrossinek commented Jan 16, 2023

Summary

This class is designed to unify the usage of tensors throught the stack. It provides a central entry point for the handling of arrays enabling seamless interchange of dense and sparse arrays in any usecase. This is done by implementing this class as an np.ndarray container as described here. At the same time this class can also wrap a sparse.SparseArray (which in turn implements the np.ndarray interface).

Details and comments

I have two major motivators for this proposal:

  1. one is explained in more detail in Improve the handling of label_template during SparseLabelOp.from_polynomial_tensor #1032
  2. the other one is the implementation of AO-symmetry aware storage containers for 2-body integrals. I already have some prototypes prepared offline which will allow us to handle 1-, 4- and 8-fold symmetric 2-body terms while they can pretend to be fully functional 4-D matrices for the purposes of expanding them into our Hamiltonian terms (WIP code available here). These classes will also allow us to greatly simplify the logic in our FCIDump parser, effectively dealing with Improve FCIDump parser #933.

@coveralls
Copy link
Copy Markdown

coveralls commented Jan 17, 2023

Pull Request Test Coverage Report for Build 4148330521

  • 197 of 231 (85.28%) changed or added relevant lines in 5 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.0007%) to 85.934%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_nature/settings.py 18 23 78.26%
qiskit_nature/second_q/operators/tensor.py 141 170 82.94%
Files with Coverage Reduction New Missed Lines %
qiskit_nature/second_q/operators/spin_op.py 1 83.33%
qiskit_nature/second_q/operators/polynomial_tensor.py 2 87.43%
Totals Coverage Status
Change from base Build 4146231983: -0.0007%
Covered Lines: 17912
Relevant Lines: 20844

💛 - Coveralls

@mrossinek mrossinek mentioned this pull request Jan 17, 2023
5 tasks
@mrossinek mrossinek requested a review from ElePT January 24, 2023 08:36
Copy link
Copy Markdown
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

Hi! I think I will probably have to take another look at the PR to fully understand the uses of this new class. So far, I only have a question: I assume there is a good reason for this, but I wonder... why did you choose not to wrap Numbers into non-dimensional np.arrays from the start? I feel like this could simplify the code, but I probably don't have clear enough vision of how numpy works in these cases.

@mrossinek
Copy link
Copy Markdown
Member Author

Hi! I think I will probably have to take another look at the PR to fully understand the uses of this new class. So far, I only have a question: I assume there is a good reason for this, but I wonder... why did you choose not to wrap Numbers into non-dimensional np.arrays from the start? I feel like this could simplify the code, but I probably don't have clear enough vision of how numpy works in these cases.

Thanks for raising this! I am actually no longer aware of why I tried so hard to achieve this.. 🤔 I just pushed a commit to revert this and see how this goes. This is actually what we were already doing in the PolynomialTensor, too 👍

Copy link
Copy Markdown
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.

In looking at the intent for this with symmetry integrals this looks like a great thing to have to simplify general usage of such in the stack here.

Needs a release note though I think.

Comment thread qiskit_nature/second_q/operators/polynomial_tensor.py Outdated
Comment thread qiskit_nature/second_q/operators/tensor.py
This matches what we did in the PolynomialTensor and avoids unnecessary
complicated code to deal with Number objects where otherwise one always
encounters an array.
The Jupyter Sphinx integration is currently unable to properly suppress
warnings resulting in prints to stderr [1]. This causes the docs to fail
building properly.
Qiskit Terra already removed the usage of `jupyter-execute` statements a
while ago [2], so we are following suit in this regard, too.

[1]: jupyter/jupyter-sphinx#178
[2]: Qiskit/qiskit#9346
@mergify mergify Bot merged commit 5d8b887 into qiskit-community:main Feb 10, 2023
@mrossinek mrossinek deleted the tensor branch February 21, 2023 16:08
Anthony-Gandon pushed a commit to Anthony-Gandon/qiskit-nature that referenced this pull request May 25, 2023
* Implement the Tensor class

* Always wrap scalar numbers into numpy array

This matches what we did in the PolynomialTensor and avoids unnecessary
complicated code to deal with Number objects where otherwise one always
encounters an array.

* Add qiskit_nature.settings.tensor_wrapping

* Add release note

* fix: remove jupyter-execute blocks

The Jupyter Sphinx integration is currently unable to properly suppress
warnings resulting in prints to stderr [1]. This causes the docs to fail
building properly.
Qiskit Terra already removed the usage of `jupyter-execute` statements a
while ago [2], so we are following suit in this regard, too.

[1]: jupyter/jupyter-sphinx#178
[2]: Qiskit/qiskit#9346

* fix: rework settings.tensor_wrapping as settings.tensor_unwrapping

* docs: update documentaiton of PolynomialTensor now that Tensor exists

---------

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants