Skip to content

Simpler exp val#156

Open
aeddins-ibm wants to merge 13 commits intomainfrom
simpler-exp-val
Open

Simpler exp val#156
aeddins-ibm wants to merge 13 commits intomainfrom
simpler-exp-val

Conversation

@aeddins-ibm
Copy link
Collaborator

Main changes are in the first commit. Slightly edited version of its commit message:

Previously, first averaged over shots, then averaged over twirls (or other specified axes).

This was complicated by the need to keep track of how many shots were accepted by postselection in order to propagate uncertainty properly later when averaging over twirls. Also by the need to keep track of PEC signs (pauli_signs).

Now, we first flatten avg_axis (typically the twirl axis) into the shots axis, then do all averaging + postselection at once. This avoids needing to work with weights. This should also avoid the situation where postselection rejects all shots on a single twirl, resulting in nan for the average of all twirls (particularly problematic when approaching the theoretically-optimal case of 1 shot per twirl).

Previously, first averaged over shots, then averaged over twirls (or other specified axes).

This was complicated, because we needed to keep track of how many shots were accepted by postselection in order to propagate uncertainty properly later when averaging over twirls. Likewise we needed to keep track of PEC signs (`pauli_signs`).

Now, we first flatten `avg_axis` (typically the twirl axis) into the shots axis, then do all averaging + postselection at once. This avoids needing to work with `weights`. This also avoids the situation where postselection rejects all shots on a single twirl, resulting in `nan` for the average of all twirls (particularly problematic when approaching the theoretically-optimal case of 1 shot per twirl).
`np.any` seems like a more readable version of `np.logical_or.reduce` here.

The `np.asarray` call seems to be vestigial.
@coveralls
Copy link

coveralls commented Mar 11, 2026

Pull Request Test Coverage Report for Build 23020221462

Details

  • 20 of 23 (86.96%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.4%) to 90.244%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_addon_utils/exp_vals/expectation_values.py 20 23 86.96%
Totals Coverage Status
Change from base Build 22785714859: -0.4%
Covered Lines: 740
Relevant Lines: 820

💛 - Coveralls

Copy link
Collaborator

@caleb-johnson caleb-johnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, thanks!

Replace a few lines in the optional, empirical estimate of `gamma` with a single equivalent line.

Reminder that this empirical estimate of `gamma` is still unreliable as it can diverge or be negative.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants