Open pdhoolia opened 3 weeks ago
To address the issue of num_unique_paulis
falsely reporting as 0
, we can implement the first suggested solution. This approach involves initializing num_unique_paulis
with the current length of each observable at the start of the backpropagation process. This ensures that we have a correct initial count of unique Pauli terms, even if no gates in a slice overlap with the observable.
Here's how we can implement this solution in the backpropagation.py
file:
backpropagation.py
Initialize num_unique_paulis
with the current length of each observable:
Update the initialization of num_unique_paulis
in the SliceMetadata
to reflect the current length of each observable. This ensures that we have a correct initial count of unique Pauli terms.
# Modify the initialization of SliceMetadata within the backpropagation function
slice_metadata = SliceMetadata(
slice_errors=[0.0] * num_observables,
raw_num_paulis=[0] * num_observables,
num_unique_paulis=[len(op) for op in observables_tmp] if operator_budget.simplify else None,
num_duplicate_paulis=[0] * num_observables if operator_budget.simplify else None,
num_trimmed_paulis=[0] * num_observables if operator_budget.simplify else None,
sum_trimmed_coeffs=[0] * num_observables if operator_budget.simplify else None,
num_truncated_paulis=[0] * num_observables,
num_paulis=[0] * num_observables,
sum_paulis=None,
num_qwc_groups=None,
)
Initialization with len(op)
: We initialize num_unique_paulis
with the length of each observable in observables_tmp
. This ensures that even if no gates in a slice overlap with the observable, the num_unique_paulis
field will reflect the correct number of unique Pauli terms.
Avoiding Redundant Checks: By setting the initial value correctly, we avoid the need for additional checks or updates later in the backpropagation process, making the code cleaner and more efficient.
This approach should resolve the issue without introducing significant overhead, as the length computation is straightforward and efficient. It also aligns with the suggestion that this solution is cleaner and should not pose any performance concerns.
I think this a very nice and well explained comment. My only comment will be that I would not have written the entire code corresponding to SliceMetadata
here. Rather I would have just shown the change of the line required (as suggested in the original issue). This is because SliceMetadata
may have been much much bigger, and writing the whole thing will make it difficult to detect the change. Another possible way may be to write
slice_metadata = SliceMetadata(
...
num_unique_paulis=[len(op) for op in observables_tmp] if operator_budget.simplify else None,
...
)
This acknowledges the existence of other variables in SliceMetadata, but just focuses on the change required.
Thank you for the feedback. You're right; focusing only on the specific change makes it easier to identify what needs to be modified, especially in larger code blocks. Here's the refined suggestion focusing on the specific change:
backpropagation.py
Modify the initialization of num_unique_paulis
:
Update the initialization of num_unique_paulis
within the SliceMetadata
to reflect the current length of each observable.
slice_metadata = SliceMetadata(
...
num_unique_paulis=[len(op) for op in observables_tmp] if operator_budget.simplify else None,
...
)
SliceMetadata
without listing them all. This approach is concise and makes it easy to see the exact modification required.This change will ensure that num_unique_paulis
is initialized correctly, avoiding the issue of it falsely reporting as 0
.
What is happening and why is it wrong?
The
num_unique_paulis
field tracked by the metadata when operators are being simplified can falsely report to be0
. This does not make any sense because it would imply an empty operator has occurred but we know for a fact that the smallest operator size is bound by 1 (when it gets reduced to an identity operator).How can we reproduce the issue?
This will output
0
even though the obvious correct number of unique Pauli terms should be1
(as shown by the second print line).Any suggestions?
The reason that this occurs is the following: when none of the gates in a slice of the circuit overlap with the observable being backpropagated, the
num_unique_paulis
value never gets set to a correct value and, thus, remains to be its initialized value.Specifically, the following lines have to always execute: https://github.com/pdhoolia/qiskit-addon-obp/blob/1cb1f3796d98c28a8332b97b2f03f2a3acfa8833/qiskit_addon_obp/backpropagation.py#L179-L181
Resulting in the 0 remaining set from here: https://github.com/pdhoolia/qiskit-addon-obp/blob/1cb1f3796d98c28a8332b97b2f03f2a3acfa8833/qiskit_addon_obp/backpropagation.py#L150
We have multiple solutions for how to fix this.
Change the initialization of
num_unique_paulis
to be the following:This will correctly initialize the num_unique_paulis with the current length of each observable.
Add a check here whether
num_unique_paulis
is still zero and, if so, fix it at this point.Personally, I find the first fix cleaner but it adds an additional loop over the observables (which should not be a problem). The latter fix would be preferable if any of the other metadata fields also need to be handled specially, but looking at it now, those should all be correct when initialized with
0
(becausenum_paulis
will always get updated correctly here). But it won't hurt to double check whether this is indeed correct.I will leave it up to you with which solution you will go 👍