Fix SparsePauliOp initialization with dense Y labels

[trigpolynom]

Summary

Fixes #13522

"When a SparsePauliOp object is instantiated with a long string of Y's (greater than 100), the imaginary component of the coefficients is non-zero (but trivially small, ~1e-15). This results in errors when submitting jobs using qiskit-ibm-runtime which throws an error when using the SparsePauliOp as the expectation value to estimate."

The fix to the above problem involved using modular arithmetic to calculate coeffs for the SparsePauliOp when coeffs is None. This avoids the floating point error that occurs for instantiating heavy-weight SparsePauliOps.

Details and comments

Added a test method test_sparse_pauli_op_init_long_ys to ensure that the error causing PauliList ('Y"*100) results with the same coeffs as PauliList ("Y"*99)

[coveralls]

Pull Request Test Coverage Report for Build 12424985899

Details

• 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
• 11 unchanged lines in 3 files lost coverage.
• Overall coverage decreased (-0.004%) to 88.947%

---

Files with Coverage Reduction	New Missed Lines	%
crates/accelerate/src/unitary_synthesis.rs	1	92.2%
crates/qasm2/src/lex.rs	4	91.98%
crates/qasm2/src/parse.rs	6	97.15%

Totals
Change from base Build 12420636821:	-0.004%
Covered Lines:	79426
Relevant Lines:	89296

---

💛 - Coveralls

[ShellyGarion]

Hi @trigpolynom - thanks for fixing this bug!
After you complete the tests and fix the lint error, could you please add release notes?

[Cryoris]

That's a nice solution! Couldn't we avoid using p here and just adjust the lookup accordingly?

Suggested change

	p = (3 * exp_val) % 4 # (−i)^(phase - count_y) = i^(3*(phase - count_y))
	lookup = np.array([1 + 0j, 1j, -1 + 0j, -1j], dtype=coeffs.dtype)
	lookup = np.array([1, -1j, -1j, 1j], dtype=coeffs.dtype) # (-1j) ** [0, 1, 2, 3]

[trigpolynom]

Oh I like this approach much better thank you! See updated code.

[Cryoris]

Also a tiny nitpick -- exp_val to me sounds like an expectation value, as we're in the realm of quantum computing. Would just exponent be a more precise name? 🙂

[trigpolynom]

yes makes complete sense. See update.

[ShellyGarion]

note that there is still a lint error (see here)

************* Module qiskit.quantum_info.operators.symplectic.sparse_pauli_op
qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py:171:0: C0301: Line too long (107/105) (line-too-long)

[trigpolynom]

Hi @ShellyGarion and @Cryoris thanks for taking a look at this! I'll make the updates and requested changes tonight EST.

[qiskit-bot]

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 following people are relevant to this code:

• @Qiskit/terra-core

[Cryoris]

We don't need a subTest since there's only a single assertion statement in the function, also I think it would be better to use a fixed comparison value -- theoretically both 99 and 100 Y terms could be equal but wrong 🙂 We could e.g. just do

Suggested change

	with self.subTest(msg="make SparsePauliOp from string of Y's"):
	yyy = SparsePauliOp("Y" * 100)
	print(yyy.coeffs)
	yy = SparsePauliOp("Y" * 99)
	print(yy.coeffs)
	self.assertEqual(yyy.coeffs, yy.coeffs)
	y = SparsePauliOp("Y" * 1000)
	self.assertEqual(1, y.coeffs[0])

[Cryoris]

Thanks for adding a reno! Note that we don't typically explain how the bug was fixed if it is a technical detail. However we do include the issue that has been fixed, so users can have a look at more detailed discussions, if there are any.

Suggested change

	Fixed a bug where a floating point arithmetic error caused a heavy-weight
	initialization of :class:`.SparsePauliOp` (pauli list of size > 99) without any
	specified coeffs, to have non-zero imaginary components. The fix involved using
	modular arithmetic to set the correct coefficients based on the phase and count_y of
	the :class:`.PauliList`.
	Fixed a bug where a initializing :class:`.SparsePauliOp` with a large
	number of Pauli-``Y`` terms (typically :math:`\geq 100`) and no explicit
	``coeffs`` would result in a coefficient close to 1 but with a floating point
	error. The coefficient is now correctly 1 per default.
	Fixed `#13522 <https://github.com/Qiskit/qiskit/issues/13522>`__.