Added argsort and sort method to SparsePauliOp #8016

kUmezawa · 2022-05-03T08:41:19Z

Summary

This is an update as part of QAMP's activities.
Relating issue: qiskit-advocate/qamp-spring-22#20
PauliList has argsort() method and sort() method.
We need these features in SparsePauliOp.

Details and comments

argsort() method returns the composition of permutations in the order of sorting by coefficient and sorting by Pauli.

sort() method returns a new sorted SparsePauliOp class. After sorting the coefficients using numpy's argsort, sort by Pauli.

qiskit-bot · 2022-05-03T08:41:22Z

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

@Qiskit/terra-core
@ikkoham

kUmezawa · 2022-05-03T08:42:11Z

@ikkoham
Please review. Thank you.

coveralls · 2022-05-05T06:45:46Z

Pull Request Test Coverage Report for Build 2654643569

8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.003%) to 83.996%

Totals
Change from base Build 2651587373:	0.003%
Covered Lines:	55829
Relevant Lines:	66466

💛 - Coveralls

HuangJunye · 2022-05-05T08:41:30Z

@kUmezawa Is this part of QAMP? If so can you please mention the QAMP project in the description? I will also ad QAMP label to the PR. Thank you.

kUmezawa · 2022-05-09T01:01:37Z

@HuangJunye

Is this part of QAMP? If so can you please mention the QAMP project in the description?

Yes, this is part of QAMP. I have updated this pr description. Thank you.

ikkoham

Thank you. Originally, the simplify method sorted as out of spec, but since 0.20.0 it no longer sorts, so this method is useful.

Basically, It looks good to me, but release note is required for new feature. Could you please add the release note?

kUmezawa · 2022-05-09T08:31:29Z

@ikkoham
Is it okay to add a new yml file to releasenotes / notes / 0.20?

t-imamichi · 2022-05-09T09:14:56Z

There is an attempt to parametrize coefficients of SparsePauliOp #7215.
Does weight option of this PR work fine with parameter expressions as well as complex values?

t-imamichi · 2022-05-09T09:16:08Z

As for release note, please check out this documentation.
https://github.com/Qiskit/qiskit-terra/blob/main/CONTRIBUTING.md#release-notes

kUmezawa · 2022-05-10T04:20:38Z

@t-imamichi

Does weight option of this PR work fine with parameter expressions as well as complex values?

I haven't considered it yet.
@ikkoham Let's discuss it in QAMP.

As for release note, please check out this documentation.

Thank you.
I follow the steps to create the release notes

kUmezawa · 2022-05-12T07:32:34Z

@t-imamichi
I consulted with Hamamura-sann
Since the Parameter has nothing to do with this weight, we judge that it is not necessary to consider it this time.

ikkoham · 2022-05-13T10:25:54Z

Thanks. I'll have to think about whether we handle the error or simply just get a TypeError from NumPy when introducing Parameter for SparsePauliOp. I'll keep this in my mind.

Cryoris

Also from my side the code LGTM! But what is the motivation for adding these methods? That wasn't quite clear from the PR description 🙂

I'm quite confused by the weight argument -- I thought it was related to the coefficients and only when looking at PauliList I realized that you define the weight as the number of non-identity Paulis. In the context of the PauliList I see how that makes sense but since we have coefficients in the SparsePauliOp I think this naming might be a bit unclear. Could we maybe use a more precise argument?

releasenotes/notes/0.20/add-sparsepauliop-methods-00a7e6cc7055e1d0.yaml

test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py

qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py

kUmezawa · 2022-05-18T00:38:03Z

@Cryoris

Thank you for your review.

Also from my side the code LGTM! But what is the motivation for adding these methods? That wasn't quite clear from the PR description

I'm sorry that the motive is difficult to understand. As Mr. Hamamura wrote, the simplify method cannot be sorted, so the motivation was to prepare an alternative method. I will specify it in the release notes.

Could we maybe use a more precise argument?

OK. I will consider it.

kUmezawa · 2022-07-06T06:28:21Z

@Cryoris @ikkoham
Sorry for the late response.
The release notes have been fixed.
Added test code for empty list.
Added sample code to the documentation.

Thank you.

qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py

Cryoris · 2022-07-07T11:59:23Z

qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py

+
+        Args:
+            weight (bool): optionally sort by weight if True (Default: False).
+            This argument is the same as weight of argsort method of PauliList


Suggested change

This argument is the same as weight of argsort method of PauliList

This argument is the same as weight of argsort method of :class:`.PauliList`.

qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py

Cryoris · 2022-07-07T12:00:16Z

qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py

+
+        Args:
+            weight (bool): optionally sort by weight if True (Default: False).
+            This argument is the same as weight of sort method of PauliList


See comment on the documentation of argsort 🙂

releasenotes/notes/0.21/add-sparsepauliop-methods-00a7e6cc7055e1d0.yaml

test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py

kUmezawa · 2022-07-08T08:33:36Z

@Cryoris @ikkoham
Thank you for your review.

Could you add a test for a corner case like an empty SparsePauliOp to make sure no errors are being raised?

Shouldn't sorting on an empty set work? I would argue since it's empty it's already "sorted" anyways

I understand that the meaning of your advice is to point out what happens when you sort SparcePauliOp with an empty list. Is that wrong?

See comment on the documentation of argsort

I will change the comment sort to argsort.

Cryoris · 2022-07-08T08:44:47Z

I understand that the meaning of your advice is to point out what happens when you sort SparcePauliOp with an empty list. Is that wrong?

I mean that right now it looks like sorting an empty SparsePauliOp raises an error. But I think it should not raise an error and just return the empty SparsePauliOp 🙂

kUmezawa · 2022-07-08T09:01:00Z

@Cryoris
Thank you.

I mean that right now it looks like sorting an empty SparsePauliOp raises an error. But I think it should not raise an error and just return the empty SparsePauliOp

I get an error when generating a SparcePauliOp class with an empty list.
I don't think it's possible to sort an empty SparcePauliOp.

Cryoris · 2022-07-08T14:36:47Z

Oh fair enough! Then of course sorting doesn't work. Thanks for trying! 🙂

kUmezawa · 2022-07-12T01:02:44Z

@Cryoris
Thank you.

＞Oh fair enough! Then of course sorting doesn't work. Thanks for trying!

OK. Thank you.

＞See comment on the documentation of argsort

Added description of arguments.

ikkoham · 2022-07-12T06:43:37Z

right. empty SparsePauliOp is not allowed.

Cryoris

LGTM thanks for the updates @kUmezawa!

ikkoham

LGTM. Thank you for your contribution!

kUmezawa added 5 commits April 28, 2022 01:02

first share

7edfaca

Remove reverse option

820f315

Apply Lint

15df641

Modify wording

cbb04b8

Merge remote-tracking branch 'upstream/main' into feature_sparsepauliop

b099402

kUmezawa requested review from a team and ikkoham as code owners May 3, 2022 08:41

kUmezawa mentioned this pull request May 3, 2022

Implement new features and improve documentation in Operators qiskit-advocate/qamp-spring-22#20

Open

ikkoham added Changelog: New Feature Include in the "Added" section of the changelog mod: quantum info Related to the Quantum Info module (States & Operators) labels May 5, 2022

ikkoham added the QAMP issues and PRs relating to Qiskit Advocates Mentorship Program label May 9, 2022

ikkoham suggested changes May 9, 2022

View reviewed changes

kUmezawa added 2 commits May 12, 2022 16:48

add release note

9d0cb85

Merge remote-tracking branch 'upstream/main' into feature_sparsepauliop

68442ee

Cryoris reviewed May 13, 2022

View reviewed changes

releasenotes/notes/0.20/add-sparsepauliop-methods-00a7e6cc7055e1d0.yaml Outdated Show resolved Hide resolved

test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py Show resolved Hide resolved

qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py Show resolved Hide resolved

HuangJunye added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jun 21, 2022

kUmezawa added 2 commits July 6, 2022 14:00

Merge remote-tracking branch 'upstream/main' into feature_sparsepauliop

76a9228

Correct the text

dfe7d27

Correct the text

e2b795e

Delete words

94116dc

Cryoris reviewed Jul 7, 2022

View reviewed changes

qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py Outdated Show resolved Hide resolved

Cryoris reviewed Jul 7, 2022

View reviewed changes

qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py Outdated Show resolved Hide resolved

Cryoris reviewed Jul 7, 2022

View reviewed changes

releasenotes/notes/0.21/add-sparsepauliop-methods-00a7e6cc7055e1d0.yaml Outdated Show resolved Hide resolved

Cryoris reviewed Jul 7, 2022

View reviewed changes

test/python/quantum_info/operators/symplectic/test_sparse_pauli_op.py Show resolved Hide resolved

kUmezawa added 2 commits July 12, 2022 09:47

Merge remote-tracking branch 'upstream/main' into feature_sparsepauliop

35dc1e7

Added description of arguments

b6f8f08

Fix typos & update reno

ad74902

Cryoris approved these changes Jul 12, 2022

View reviewed changes

ikkoham approved these changes Jul 12, 2022

View reviewed changes

ikkoham added the automerge label Jul 12, 2022

mergify bot merged commit 925d8cb into Qiskit:main Jul 12, 2022

jakelishman mentioned this pull request Dec 21, 2023

[WIP] Implement sort method for SparsePauliOp #6402

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added argsort and sort method to SparsePauliOp #8016

Added argsort and sort method to SparsePauliOp #8016

kUmezawa commented May 3, 2022 •

edited

Loading

qiskit-bot commented May 3, 2022

kUmezawa commented May 3, 2022

coveralls commented May 5, 2022 •

edited

Loading

HuangJunye commented May 5, 2022

kUmezawa commented May 9, 2022 •

edited

Loading

ikkoham left a comment

kUmezawa commented May 9, 2022

t-imamichi commented May 9, 2022

t-imamichi commented May 9, 2022

kUmezawa commented May 10, 2022

kUmezawa commented May 12, 2022

ikkoham commented May 13, 2022

Cryoris left a comment

kUmezawa commented May 18, 2022

kUmezawa commented Jul 6, 2022

Cryoris Jul 7, 2022

Cryoris Jul 7, 2022

kUmezawa commented Jul 8, 2022 •

edited

Loading

Cryoris commented Jul 8, 2022

kUmezawa commented Jul 8, 2022

Cryoris commented Jul 8, 2022

kUmezawa commented Jul 12, 2022

ikkoham commented Jul 12, 2022

Cryoris left a comment

ikkoham left a comment

	This argument is the same as weight of argsort method of PauliList
	This argument is the same as weight of argsort method of :class:`.PauliList`.

Added argsort and sort method to SparsePauliOp #8016

Added argsort and sort method to SparsePauliOp #8016

Conversation

kUmezawa commented May 3, 2022 • edited Loading

Summary

Details and comments

qiskit-bot commented May 3, 2022

kUmezawa commented May 3, 2022

coveralls commented May 5, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2654643569

💛 - Coveralls

HuangJunye commented May 5, 2022

kUmezawa commented May 9, 2022 • edited Loading

ikkoham left a comment

Choose a reason for hiding this comment

kUmezawa commented May 9, 2022

t-imamichi commented May 9, 2022

t-imamichi commented May 9, 2022

kUmezawa commented May 10, 2022

kUmezawa commented May 12, 2022

ikkoham commented May 13, 2022

Cryoris left a comment

Choose a reason for hiding this comment

kUmezawa commented May 18, 2022

kUmezawa commented Jul 6, 2022

Cryoris Jul 7, 2022

Choose a reason for hiding this comment

Cryoris Jul 7, 2022

Choose a reason for hiding this comment

kUmezawa commented Jul 8, 2022 • edited Loading

Cryoris commented Jul 8, 2022

kUmezawa commented Jul 8, 2022

Cryoris commented Jul 8, 2022

kUmezawa commented Jul 12, 2022

ikkoham commented Jul 12, 2022

Cryoris left a comment

Choose a reason for hiding this comment

ikkoham left a comment

Choose a reason for hiding this comment

kUmezawa commented May 3, 2022 •

edited

Loading

coveralls commented May 5, 2022 •

edited

Loading

kUmezawa commented May 9, 2022 •

edited

Loading

kUmezawa commented Jul 8, 2022 •

edited

Loading