Skip to content

Move noise sampling to pulser-core #916

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

Draft
wants to merge 9 commits into
base: develop
Choose a base branch
from
Draft

Conversation

a-corni
Copy link
Collaborator

@a-corni a-corni commented Aug 7, 2025

Currently, each emulator has to implement its custom noise sampling. They are generally inspired from pulser-simulation. If we want to support a new noise type, we have to implement it in each emulator. This proposes to move the tools to generate samples with noise to pulser-core.

@a-corni a-corni requested a review from a team as a code owner August 7, 2025 16:38
@a-corni a-corni requested a review from kbidzhiev August 7, 2025 16:39
@a-corni
Copy link
Collaborator Author

a-corni commented Aug 8, 2025

For this changes to be relevant, it was convened the final object should contain:

  • The noiseless samples (for plotting)
  • The noise trajectory (for plotting with the noise, which is a desired new feature)
  • The interaction somehow (either the register positions, or the interaction matrix directly)
  • information about what type of interaction term to do (rydberg, XY, etc.)
  • information about SLM mask, because the interaction matrix can change because of it

@HGSilveri HGSilveri marked this pull request as draft August 18, 2025 12:06
@HGSilveri HGSilveri changed the title [Draft] Move noise sampling to pulser-core Move noise sampling to pulser-core Aug 18, 2025
Copy link
Collaborator

@HGSilveri HGSilveri left a comment

Choose a reason for hiding this comment

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

Just a preliminary consideration on this, I'd love to discuss it further when we get the chance

from pulser.sequence import Sequence


class BaseHamiltonian:
Copy link
Collaborator

Choose a reason for hiding this comment

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

At a glance, it sounds more natural to me that this would be called HamiltonianData and given to Hamiltonian, rather than its base class. Of course, the design of the class itself would have to be adapted to this paradigm.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm inclined to agree here. It's not a big change to make.

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