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

Implement arithmetic on SparseObservable #13298

Merged
merged 3 commits into from
Oct 29, 2024

Conversation

jakelishman
Copy link
Member

@jakelishman jakelishman commented Oct 8, 2024

Summary

The simple arithmetic operators of addition, subtraction, multiplication, division and tensor product are in line with other quantum_info objects, including coercion from other objects (though better behaved on failed coercions, in many cases).

Where appopriate, in-place operations will re-use the existing allocations. The tensor product cannot really be done in place, so it doesn't define a special __rxor__ operator, but it inherits the standard Python behaviour of this being derived from __xor__.

There are further mathematical operations to add around composition and evolution, and to apply a TranspileLayout to the observable. All of these, in the quantum_info style either directly deal with a layout, or take a qargs argument that is effectively a sublayout for the right-hand side of the operation. These will be added in a follow-up.

Details and comments

Built on top of #12671, so depends on it.

@jakelishman jakelishman added on hold Can not fix yet Changelog: New Feature Include in the "Added" section of the changelog mod: quantum info Related to the Quantum Info module (States & Operators) Rust This PR or issue is related to Rust code in the repository mod: primitives Related to the Primitives module labels Oct 8, 2024
@jakelishman jakelishman added this to the 1.3.0 milestone Oct 8, 2024
@jakelishman jakelishman requested a review from a team as a code owner October 8, 2024 17:48
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Oct 8, 2024

Pull Request Test Coverage Report for Build 11576212029

Details

  • 375 of 410 (91.46%) changed or added relevant lines in 1 file are covered.
  • 16 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.008%) to 88.693%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/accelerate/src/sparse_observable.rs 375 410 91.46%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 4 91.98%
crates/qasm2/src/parse.rs 12 97.15%
Totals Coverage Status
Change from base Build 11572090796: 0.008%
Covered Lines: 75349
Relevant Lines: 84955

💛 - Coveralls

The simple arithmetic operators of addition, subtraction,
multiplication, division and tensor product are in line with other
`quantum_info` objects, including coercion from other objects (though
better behaved on failed coercions, in many cases).

Where appopriate, in-place operations will re-use the existing
allocations.  The tensor product cannot really be done in place, so it
doesn't define a special `__rxor__` operator, but it inherits the
standard Python behaviour of this being derived from `__xor__`.

There are further mathematical operations to add around composition and
evolution, and to apply a `TranspileLayout` to the observable.  All of
these, in the `quantum_info` style either directly deal with a layout,
or take a `qargs` argument that is effectively a sublayout for the
right-hand side of the operation.  These will be added in a follow-up.
@jakelishman jakelishman removed the on hold Can not fix yet label Oct 24, 2024
@jakelishman
Copy link
Member Author

Now rebased and ready for review.

Copy link
Contributor

@Cryoris Cryoris left a comment

Choose a reason for hiding this comment

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

This looks good to me I just have some short questions below 🙂

crates/accelerate/src/sparse_observable.rs Show resolved Hide resolved
crates/accelerate/src/sparse_observable.rs Show resolved Hide resolved
crates/accelerate/src/sparse_observable.rs Show resolved Hide resolved
This puts the `self is other` check into the `__add__` and `__sub__`
methods so that the behaviour of `x + x` is consistent with `x += x`,
with regards to the addition being done as a scalar multiplication
instead of concatentation.  Both forms are mathematically correct, but
this makes sure they're aligned.
Copy link
Contributor

@Cryoris Cryoris left a comment

Choose a reason for hiding this comment

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

LGTM!

@Cryoris Cryoris added this pull request to the merge queue Oct 29, 2024
Merged via the queue into Qiskit:main with commit ddf93ab Oct 29, 2024
16 checks passed
@jakelishman jakelishman deleted the sparse-observable-maths branch October 29, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog mod: primitives Related to the Primitives module mod: quantum info Related to the Quantum Info module (States & Operators) Rust This PR or issue is related to Rust code in the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants