enrich executor_expectation_values input options#170
enrich executor_expectation_values input options#170dekelmeirom wants to merge 2 commits intomainfrom
Conversation
add option to give (obs, bases) as input for executor_expectation_values
Coverage Report for CI Build 24664371484Coverage increased (+3.2%) to 93.426%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
dekelmeirom
force-pushed
the
obs_and_bases_input
branch
from
1fc7b9a to
da4f555
Compare
| ValueError if the number of entries in `basis_dict` does not equal the length of `bool_array` along `meas_basis_axis`. | ||
| ValueError if `meas_basis_axis` is `None` but `len(basis_mapping) != 1`. | ||
| ValueError if the number of entries in `basis_mapping` does not equal the length of `bool_array` along `meas_basis_axis`. | ||
| ValueError if the given measurement_basis can not cover all of the terms in al lof the observables. |
There was a problem hiding this comment.
| ValueError if the given measurement_basis can not cover all of the terms in al lof the observables. | |
| ValueError if the given measurement_basis can not cover all of the terms in all of the observables. |
| basis_dict: dict[Pauli, list[SparsePauliOp | None]], | ||
| bool_array: np.ndarray[tuple[int, ...], np.dtype[np.bool_]], | ||
| basis_mapping: dict[Pauli, list[SparsePauliOp | None]] | ||
| | tuple[Sequence[SparsePauliOp], Sequence[str | PauliList]], |
There was a problem hiding this comment.
Should the type hint for the tuple input be: tuple[Sequence[SparsePauliOp], Sequence[str] | PauliList], rather than what you have here?
|
|
||
|
|
||
| def find_measure_basis_to_observable_mapping( | ||
| observables: Sequence[SparsePauliOp], measure_bases: Sequence[str | int | PauliList] |
There was a problem hiding this comment.
Confused about the measure_bases type hint here too. Should it be Sequence[str | Sequence[int]] | PauliList?
There was a problem hiding this comment.
I'm also not sure how generally useful this function is. Maybe it could be a private method in the expectation_values module where it's used? We can just compress the docstring down to one line if we make it private. What do you think?
| False in observable_elements_list | ||
| for observable_elements_list in observables_elements_basis_found | ||
| ): | ||
| raise ValueError("Some observable elements do not commute with any measurement basis.") |
There was a problem hiding this comment.
We could print which observable doesn't commute with which basis here
|
|
||
| def _apply_pec_signs( | ||
| bool_array: np.ndarray[tuple[int, ...], np.dtype[np.bool]], | ||
| bool_array: np.ndarray[tuple[int, ...], np.dtype[np.bool_]], |
There was a problem hiding this comment.
Was the linter throwing an error about this? Wondering why these changes are necessary here and above
| raise ValueError( | ||
| f"{len(basis_dict) = } does not match {bool_array.shape[meas_basis_axis] = }." | ||
| ) | ||
| if isinstance(basis_mapping, dict): |
There was a problem hiding this comment.
This block of code (123-147) where we check basis mapping and get everything in order can be moved to a private method at the bottom of the module. That will clear up some space for the core code in the module. I think it'll help the readability.
3 participants
add option to give (obs, bases) as input for executor_expectation_values
in addition, add tests for
measurement_bases.pyaddresses issue #163