Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Refactoring] Noise protocols #584

Closed
wants to merge 23 commits into from
Closed

[Refactoring] Noise protocols #584

wants to merge 23 commits into from

Conversation

chMoussa
Copy link
Collaborator

Solves #583.

The idea would be to introduce three types of Noise: PulseNoise, PostProcessingNoise and BlockNoise.

  • BlockNoise are used to modify a block using the transpiler function set_noise introduced in [Feature] Add digital noise from PyQ #563.

  • PostProcessingNoise would be only used to modify the output of a program. At the moment it only includes readout, which modifies the output samples. At the moment, it is backend independent.

  • PulseNoise used to create a SimConfig instance for the Pulser backend.

There are two approaches to the interface I can think of.

  1. The first easier option would be to keep the Noise dataclass, just adding a type to it, so this does not modify much the current codebase. We can introduce the three types as children. Noise would only serve the purpose of holding options and protocol name besides for PostProcessingNoise where we extract a method add_noise from a module to be used in apply_noise which is called in a few backends.

  2. The second one would require more changes as it means making consistent which noises can be used at the backend level and the method called. For instance, the pulser backend can only accept a PulseNoise in the run method, and all backends can accept a PostProcessingNoise in the sample method. A problem we can foresee is consistent typing with base/abstract classes. Serialization may be also challenging.

@chMoussa chMoussa added the refactoring Refactoring of legacy code label Oct 16, 2024
@chMoussa chMoussa self-assigned this Oct 16, 2024
Copy link
Collaborator

@jpmoutinho jpmoutinho left a comment

Choose a reason for hiding this comment

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

Thanks @chMoussa :)

IMO, my feeling is that this separation could create some confusion. For example:

  • You suggest BlockNoise to represent the noise models that go to PyQ, but the Qadence design principle is that all programs are composed of "blocks", even ones that compile to the Pulser backend (or will compile in the future).
  • The PulseNoise is defined as specifically creating a SimConfig, but we also have time-dependent blocks that go to PyQ, and in principle this PulseNoise could also have an implementation there.
  • PostProcessingNoise -- There's not really post processing going on yet right? I feel like post-processing is what you would do then for noise mitigation.

Maybe something like:

  • DigitalNoise: for noise protocols described as discrete events acting on a block (that also represents a discrete operation). In PyQ it's essentially what is already implemented. In the future when we have digital operations in other backends (including pulser or a pulser-like backend), these can have an implementation there as well.
  • AnalogNoise: for noise protocols described as a continuous effect that affects some other continuous operation. For Pulser it can plug into the SimConfig like you said, while for PyQ we still need to implement it, but from my understanding this would be about including some extra terms in the Hamiltonian evolution right?
  • ReadoutNoise: what you called the "post processing" one

In principle, I think the set_noise function could be available for both, but it would fail if we try to do set_noise of a AnalogNoise on a digital gate, and possibly fail if we try to set a digital noise on a time-dependent hamiltonian evolution.

Wdyt?

docs/tutorials/realistic_sims/mitigation.md Outdated Show resolved Hide resolved
qadence/types.py Outdated Show resolved Hide resolved
@chMoussa
Copy link
Collaborator Author

Thanks @chMoussa :)

IMO, my feeling is that this separation could create some confusion. For example:

  • You suggest BlockNoise to represent the noise models that go to PyQ, but the Qadence design principle is that all programs are composed of "blocks", even ones that compile to the Pulser backend (or will compile in the future).
  • The PulseNoise is defined as specifically creating a SimConfig, but we also have time-dependent blocks that go to PyQ, and in principle this PulseNoise could also have an implementation there.
  • PostProcessingNoise -- There's not really post processing going on yet right? I feel like post-processing is what you would do then for noise mitigation.

Maybe something like:

  • DigitalNoise: for noise protocols described as discrete events acting on a block (that also represents a discrete operation). In PyQ it's essentially what is already implemented. In the future when we have digital operations in other backends (including pulser or a pulser-like backend), these can have an implementation there as well.
  • AnalogNoise: for noise protocols described as a continuous effect that affects some other continuous operation. For Pulser it can plug into the SimConfig like you said, while for PyQ we still need to implement it, but from my understanding this would be about including some extra terms in the Hamiltonian evolution right?
  • ReadoutNoise: what you called the "post processing" one

In principle, I think the set_noise function could be available for both, but it would fail if we try to do set_noise of a AnalogNoise on a digital gate, and possibly fail if we try to set a digital noise on a time-dependent hamiltonian evolution.

Wdyt?

Hi @jpmoutinho,

Thanks for these clarifications. It helps. On analog noise, indeed it would be just setting an option in the solvers with the list of operators. For Postprocessing (only happening in the sample methods of the backends), I thought that one could add modules if there was a need for it later. But right now we can restrict to ReadoutNoise. I'll try to adapt your suggestions rn

@RolandMacDoland
Copy link
Collaborator

I agree with @jpmoutinho on this. It makes more sense to talk about digital and analog noises in this context. As for the readout noise, I tend to think this should go in the backends too but it is currently not supported in Pulser unless I'm wrong. This is why it still remains in Qadence and induces confusion and difficulties in having a clean interface. Maybe this is something we can ask the Pulser people to implement ? It'll make our life much easier though. Wdyt ?

@chMoussa
Copy link
Collaborator Author

I agree with @jpmoutinho on this. It makes more sense to talk about digital and analog noises in this context. As for the readout noise, I tend to think this should go in the backends too but it is currently not supported in Pulser unless I'm wrong. This is why it still remains in Qadence and induces confusion and difficulties in having a clean interface. Maybe this is something we can ask the Pulser people to implement ? It'll make our life much easier though. Wdyt ?

If it is possible for them to work on this feature on their side, then I am fine by it and to put it in Pyqtorch. It would be a matter of instantiating the right object.

@chMoussa
Copy link
Collaborator Author

Closes as #591 supersedes this.

@chMoussa chMoussa closed this Oct 25, 2024
@chMoussa chMoussa deleted the cm/noise_protocols branch October 31, 2024 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactoring of legacy code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants