Skip to content

[WIP] Add simulation-based integration test for the correctness of the estimator - #3214

Draft
TsafrirA wants to merge 1 commit into
Qiskit:mainfrom
TsafrirA:ta-noiseless
Draft

[WIP] Add simulation-based integration test for the correctness of the estimator#3214
TsafrirA wants to merge 1 commit into
Qiskit:mainfrom
TsafrirA:ta-noiseless

Conversation

@TsafrirA

Copy link
Copy Markdown
Collaborator

Summary

Part of #3209 (also draws a clear separation between the tests of #3210)

AI/LLM disclosure

  • I didn't use LLM tooling, or only used it privately.

resilience_level=0,
experimental={
"local_mode": True,
# "simulator_options": ExperimentalSimulatorOptions(seed_simulator=42),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

With a random seed the test fails about half of the times. This is a bit more than I expected, I will go over the math again.

In any case, I think we should pick a passing seed, and aim for a relatively high failure rate with a random seed - it means the test is on the very edge, and would detect regressions well.

)
self.tolerance = 2 # In terms of stansdard deviations

def test_vanilla_correctness(self):

@SamFerracin SamFerracin Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this test is a bit too optimistic when it comes to testing precision. Regardless of what our docs say or do not say (and we can improve that), given well-established statistical arguments, all we can guarantee is a precision p that is O(n_shots**-0.5). Any attempt at finding a constant C such that the errors are below C/n_shots**0.5 is doomed to fail, either because the test becomes too strict and fails often or because it is too lenient and tests nothing [which echoes things we said today].

In the absence of noise, we can make two promises:

  • Our estimates are unbiased
  • If we increase n_shots, our evs get closer to the ideal values, and our stds shrink.

Therefore, I would split this test into two tests that verify those two concerns. For example, for vanilla you could have:

  • A test where you fix a precision (default?) and try to see that the expectation values are unbiased. [For example, I have tried running your test with a single assert, namely assert np.all(np.array(errors) < 0.02), without fixing the seed. I had to remove that one observable for which we expect 0, which we know is problematic, but other than that, it always passes]
  • Another test where you fix one observable, and run with 2/3 values of precision, such as 0.1, 0.01, 0.001. Then you test that what you call errors go down and that the stds go down, like Rainer did in his other test

If you go this way, an additional bonus is that you will not need seeds for the tests to pass

@SamFerracin SamFerracin Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In addition to this, you could consider parametrizing these tests over resilience levels 0, 1, and 2. So that we end up with 2 tests (times 3 -> 6 tests), and we only need to write two more for PEA and PEC

Comment on lines +151 to +160
("IIZ", np.cos(theta)),
("IYZ", np.cos(theta)),
("XIZ", np.cos(theta)),
("1IZ", 0.5 * np.cos(theta)),
("IYr", np.sin(np.pi / 4 - theta / 2) ** 2),
("X+I", 0.5),
("0IZ", 0.5 * np.cos(theta)),
("IYl", np.cos(np.pi / 4 - theta / 2) ** 2),
("X-I", 0.5),
("ZXY", 0),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks like a great test suite!

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.

2 participants