Allow empty list default initialization for SparsePauliOp

[SimoneGasperini]

Summary

Fix issue #10159, allowing the default initialization of a SparsePauliOp instance by passing no argument or passing an empty list to the static methods SparsePauliOp.from_list and SparsePauliOp.from_sparse_list.

Details and comments

When no argument or an empty list is passed, the default behaviour is to create a 0 operator (i.e. $0 \cdot \mathbb{I}$) instance (instead of throwing an unexpected error as noticed by @kevinsung). For example:

from qiskit.quantum_info import SparsePauliOp

op = SparsePauliOp.from_sparse_list()
print(op)

SparsePauliOp(['I'], coeffs=[0.+0.j])

[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 the following people are requested to review this:

• @Qiskit/terra-core
• @ikkoham

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

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.}

[jakelishman]

Thanks for looking at this. I don't think the approach you've gone with here is quite right in part because it doesn't actually solve the problem that the underlying issue was asking for, but we can see if we can move to something that does.

First: we need to verify that SparsePauliOp and the internal PauliList that are used can actually represent a zero operator. Using an identity with a zero coefficient is a possibility, but ideally it'd be represented with no entries in the table to avoid spurious work being done when interacting with one of these objects. It's quite likely that there's other places that are trying to infer the number of qubits from the first element, which is why the zero operand is so tricky. (There might be some open issues about this - it sounds familiar, but I'm not certain.)

Second: we shouldn't make the input argument optional - it's not logically something the function should do. The natural representation of an empty iterable in Python is [] or (), not omitting the argument. The original issue was just asking to support empty iterables, since those arise pretty naturally when you're constructing these operators in a programmatic manner.

The problem with empty iterables is that you can't infer from them how many qubits the operators should be defined over. It's not a good strategy to default to 1, because then the operators you're constructing most likely won't be able to interact with any non-empty ones you're constructing, so the program will likely crash later without any context.

A better strategy here would be to have from_list take an optional num_qubits argument that defaults to None (meaning to infer the value from the list). If an empty iterable is passed and num_qubits is still None, then we would error out, saying "could not determine the number of qubits from an empty list. Try passing num_qubits" or something like that. from_sparse_list already takes that argument, so it's just the extra behaviour that would be needed.

---

If you want to pursue this, I think there's definitely a plausible solution in here somewhere. You'll also need to write a few tests of the behaviour we expect, including that the code produces a human-oriented error when appropriate.

[ikkoham]

I have almost the same opinion as Jake. Note also that the representative of 0 is defined as 0 \times "I "*num_qubits in SparsePauliOp. In light of that, I personally would be fine with the current specifications, but I am not strongly opposed to making the changes discussed here.

It might be nice to support it only for from_sparse_list (not from_list).

[jakelishman]

Oh thanks, Ikko - if we already use (0, "I"*num_qubits) elsewhere, then let's stick with that in this PR. I think we should fix both from_sparse_list and from_list to support this behaviour (with from_list gaining a num_qubits argument) - it'd be weird if one supported it but the other didn't.

[SimoneGasperini]

Thanks @jakelishman and @ikkoham for all your helpful and very detailed feedbacks!
I just implemented the following changes as discussed above:

• for both from_list and from_sparse_list, the obj argument has no default value
• for both from_list and from_sparse_list, if an empty obj is passed, the 0 operator is built as ("I" * num_qubits, 0), consistently with its representation in SparsePauliOp
• from_list takes an optional num_qubits=None argument and, when it is None, its value is inferred from the input list
• if both an empty list is passed and num_qubits=None, Qiskit raises the appropriate error

Further details:

• for from_list, if num_qubits is passed and it doesn't match the objects in the input list, Qiskit raises another error
• for from_sparse_list, the num_qubits argument has no =None default value because in general is not possible to infer it by the input list (as implemented for from_list) since we are dealing with a sparse representation of pauli strings

If this is close to be the right approach, I can also work on adding the tests needed to check the functions behaviour. Let me know if any further fix/change is needed.

[SimoneGasperini]

• for both from_list and from_sparse_list, if an empty obj is passed, the 0 operator is built as ("I" * num_qubits, 0), consistently with its representation in SparsePauliOp

To be honest, I'm still not sure about the 0 SparsePauliOp representation. Look at the following examples:

op1 = SparsePauliOp(['XZ'], [0])
op2 = SparsePauliOp(['XZ']) * 0
op1.paulis  # PauliList(['XZ'])
op2.paulis  # PauliList(['II'])

The consequence of this behaviour is that op1 == op2 unexpectedly evaluates to False . But it could be even worse:

op1 = SparsePauliOp(['XZ', 'XI'], [0, 0])
op2 = SparsePauliOp(['XZ', 'XI']) * 0
op1.paulis  # PauliList(['XZ', 'XI'])
op2.paulis  # PauliList(['II'])

Correct me if I'm wrong but I would expect in both cases to get exactly the same operator. Probably it would be better to fix this before even thinking about the possibility to initialize a SparsePauliOp object from an empty list.

[ikkoham]

OK. I agree with your direction.

I'm still not sure about the 0 SparsePauliOp representation

I said simplify method gives the representative of the equivalence class of 0, e.g. SparsePauliOp(['XZ', 'XI'], [0, 0]).simplify().

[coveralls]

Pull Request Test Coverage Report for Build 5924650477

• 12 of 14 (85.71%) changed or added relevant lines in 1 file are covered.
• 4 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.03%) to 87.277%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/quantum_info/operators/symplectic/sparse_pauli_op.py	12	14	85.71%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	4	91.39%

Totals
Change from base Build 5902014500:	0.03%
Covered Lines:	74376
Relevant Lines:	85218

---

💛 - Coveralls

[SimoneGasperini]

@ikkoham @jakelishman Let me know if there is still something missing :)

[jakelishman]

Hi Simone - there seems to be some errors in the lint and test suites showing true bugs in the implementation. Could you clear those up so we can review again?

[SimoneGasperini]

Hello @jakelishman! I'm sorry about the previous errors. I cleared things up and now all the checks have passed.

[ikkoham]

It almost looks good to me. I left some minor comments.

[Cryoris]

Looking at the syntax it seems a bit unintuitive having to go via from_list or from_sparse_list to create an empty operator. Wouldn't empty = SparsePauliOp(num_qubits=n) be the more straightforward solution? (And not allow it via from_list)

[jakelishman]

Julien: the point isn't to say "this is the one way you can create an empty operator" (you can already do that by passing suitable data Numpy arrays to the SparsePaulOp constructor, no num_qubits required), it's to ensure that SparsePauliOp.from_list and .from_sparse_list have a mechanism to be able to cope when an empty list arises programmatically. Say you have a workflow that involves producing an iterable of lists of (pauli, coeff) tuples, and you want to convert that into a list of SparsePauliOp. At the moment, you need to do special manual handling internally in your loop to deal with any empty lists that might have arisen as natural ways of representing the zero operator, but this PR just adds a simple flag that makes it all work.

[jakelishman]

Thanks Simone, and sorry for the delay. This looks sensible to me now, and I'd be happy to merge it, though we should wait for @Cryoris and @ikkoham to comment as well since they had comments.

While we wait, please could you add a short "feature" release note (reno new --edit sparse-pauli-op-num-qubits)?

[jakelishman]

This looks good to me now, thanks! I'll add my tick, though we'll need to wait for @ikkoham to re-review since he's had a "request changes" review. I think you've made all the changes he suggested, though, so it should be fine.

Julien's away at the moment so probably won't be able to respond to the comments. That said, I think the main issue is about ergonomics of creating a zero operator via the default constructor, which would be a follow-up PR if there's anything needed anyway, so I think we can not wait for a response there.

[SimoneGasperini]

I accidentally closed this PR by deleting the head repository (my fork of the original Qiskit:qiskit-terra) just because I wanted to create a new fork with the new name qiskit. Is it possible for you to reopen it?

[mtreinish]

Unfortunately even as a repo admin there isn't an option to reopen the pull request because the head repositor it was opened from has been deleted. I think the only option at this point is to create a new pull request and just refer back to this as the original.

[SimoneGasperini]

Ok I see and I'm sorry. Anyway thank you for the suggestion: I'll open a new PR soon.

[jakelishman]

You'll need to open a new PR, but you can re-use the entire commit history from this one no trouble. If you don't already have it locally, you can do this:

git fetch upstream pull/10201/head:sparsepauliop_default_init

where upstream is the name of the Qiskit remote, not your fork - you might have called it something different. If you haven't configured the Qiskit-owned version as a remote, you can do

git remote add upstream https://github.com/Qiskit/qiskit

to do it (before running the command above).

After those commands, you'll have a local branch with the same name as the branch in this PR, which you can push to your new fork and then open a PR, and it'll have all the same history.

If you need it in the future: GitHub will automatically have updated the name of where your fork points to when we moved the repository, and you can actually just rename your version of it on GitHub without recreating it. You got to the repository settings for your fork, and the option's in the "General" tab.

[SimoneGasperini]

Thank you very much @jakelishman! I just opened a new PR following your instructions.

[Cryoris]

@jakelishman's comment addresses my question, I' good 🙂