T1 decay for simulator

[asmuzsoy]

When using a simulator, noise is applied after each gate. Previously, in the t1 decay method, there's only one wait gate, which means that noise is not properly applied "over time" when using a simulator. I've updated the method to add a number of wait gates proportional to the length of the delay when using a simulator.

Related to Issue #4264

As always, any feedback is appreciated!

[95-martin-orion]

I think using cirq.NoiseModel.from_noise_model_like(cirq.amplitude_damp(...)) may provide a stronger demonstration here, since the new simulator-based experiment also works with duration-agnostic noise models.

[MichaelBroughton]

I'm not really a huge fan of making the experiments require the user to provide a simulator=True/False flag that subtly changes how the experiment would be constructed in order to artificially simulate more noise.

I think the responsibility should fall on whoever made the simulator and accompanying noise model to take into consideration the increasing duration of the wait gate instead of artificially adding more waits to the experiment.

Can we leave the t1_decay code unchanged and still get the expected results on this test ? (maybe increasing the repetitions and tightening up the tolerance a bit too ?)

[95-martin-orion]

Can we leave the t1_decay code unchanged and still get the expected results on this test ?

Unfortunately, doing this right would require something along the lines of #2749, as our current noise models are agnostic to the duration of the gates involved. Making a model sensitive to WaitGate duration is easy enough, but it gets complicated when trying to make this work on a general level (how do you get the gate durations? is noise based on gate duration, or moment duration? etc.) which prompted this PR.

Essentially, the issue here is that some noise models (which may otherwise provide an accurate model of hardware) produce nonsense results here because the test assumes noise is continuous (rather than discretized). We'd like to be able to estimate T1 for these models, which requires discretizing the WaitGate.

Would changing the parameter name to something like discrete_noise be sufficient?

[MichaelBroughton]

Essentially, the issue here is that some noise models (which may otherwise provide an accurate model of hardware) produce nonsense results here because the test assumes noise is continuous (rather than discretized). We'd like to be able to estimate T1 for these models, which requires discretizing the WaitGate.

Why can't we just modify these existing noise models to check if the operation is a wait gate and then act accordingly ? Looking at the interfaces for cirq.NoiseModel it looks like it is definitely an option. Having a noise model do something different for for 10 wait gates after one another vs one longer wait gate that amounts to the same wait time seems like something we don't want our users to have to worry about and would be pretty confusing for them.

[95-martin-orion]

Why can't we just modify these existing noise models to check if the operation is a wait gate and then act accordingly ?

We can - but again, doing so correctly amounts to solving #2749. If a noise model can recognize that wait(20) and wait(40) take different amounts of time, I would also hope it knows that X(q0) and measure(q0) take different amounts of time - but exactly what those times should be and where to get them remain open design questions.

I think I understand your concern with this PR, though - it essentially "canonicalizes" the per-gate noise as a correct use case for this experiment, which is misleading. If we want to avoid that, I would support a version of this PR that only adds tests: test_noise_model_constant to demonstrate how to simulate the experiment, and a separate test to demonstrate that existing "discrete" noise models will misbehave. What do you think?

[MichaelBroughton]

Sure that makes a bit more sense to me.

[MichaelBroughton]

LGTM with one last nit.

[MichaelBroughton]

Nit: prefer https://numpy.org/doc/stable/reference/generated/numpy.testing.assert_allclose.html

[CirqBot]

Automerge cancelled: A required status check is not present.

Missing statuses: ['Build docs', 'Build protos', 'Changed files test', 'Coverage check', 'Doc test', 'Format check', 'Lint check', 'Misc check', 'Notebook formatting', 'Pytest MacOS (3.7)', 'Pytest MacOS (3.8)', 'Pytest MacOS (3.9)', 'Pytest Ubuntu (3.7)', 'Pytest Ubuntu (3.8)', 'Pytest Ubuntu (3.9)', 'Pytest Windows (3.7)', 'Pytest Windows (3.8)', 'Pytest Windows (3.9)', 'Type check', 'Typescript lint check', 'Typescript tests', 'Typescript tests coverage']