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

Fixes to the amplitude noise implementation #768

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

Conversation

HGSilveri
Copy link
Collaborator

@HGSilveri HGSilveri commented Nov 29, 2024

  • Adds information of the beam's propagation direction to Channel
  • Makes finite-waist effects depend on the position relative to the optical axis
  • Makes amplitude fluctuations occur from run to run instead of pulse to pulse
  • Fixes the transfer of laser_waist between NoiseModel and SimConfig
  • Starts adding the pulser_version to serialized devices

Note: To avoid disruptions for serialized devices between different versions, propagation_dir is defaulting to None in all channels and QutipEmulator is assuming using propagation_dir=(0,1,0) whenever it encounter None. In a future version, it would make more sense to change the default propagation_dir of Global channels to be (0,1,0) and remove this assumption from QutipEmulator.

Fixes #712

Copy link
Collaborator

@a-corni a-corni left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -78,6 +78,8 @@ class Channel(ABC):
min_avg_amp: The minimum average amplitude of a pulse (when not zero).
mod_bandwidth: The modulation bandwidth at -3dB (50% reduction), in
MHz.
propagation_dir: The propagation direction of the beam leaving the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I am not a fan of "leaving", what about

Suggested change
propagation_dir: The propagation direction of the beam leaving the
propagation_dir: The propagation direction of the beam (or wave) associated with the

@a-corni
Copy link
Collaborator

a-corni commented Nov 29, 2024

Ah I forgot about one question: should we raise a NotImplementedError if the propagation vector is implemented in a channel that is not Rydberg.Global ?

@HGSilveri
Copy link
Collaborator Author

Ah I forgot about one question: should we raise a NotImplementedError if the propagation vector is implemented in a channel that is not Rydberg.Global ?

I think that is overly restrictive, how about I do that only for Local channels?

Copy link
Collaborator

@a-corni a-corni left a comment

Choose a reason for hiding this comment

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

LGTM !

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.

Apply amplitude fluctutations at a fixed rate
2 participants