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

Add the classes for EOM mode configuration #417

Merged
merged 7 commits into from
Nov 28, 2022
Merged

Add the classes for EOM mode configuration #417

merged 7 commits into from
Nov 28, 2022

Conversation

HGSilveri
Copy link
Collaborator

  • Convert channels.py module into a module directory
  • Creates the classes for configuring operation in the EOM mode. Their usage by Sequence will come in a second PR, but is already done on branch eom-seq.

Copy link
Collaborator

@madagra madagra left a comment

Choose a reason for hiding this comment

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

I have some minor comments (mainly due to a lack of context here which can be covered offline).

The main point I do not get from this PR is how the EOM modulation is used in practice in a simulation. From what I understand now, the only things which are actually used in the Channel.modulate() method from the EOM class are the values of modulation bandwidth and rise time. I guess this will be part of the follow-up PR, right?

tests/test_eom.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@madagra madagra left a comment

Choose a reason for hiding this comment

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

I have a final comment on the unit tests, but it looks good to me after that. It might be useful to add in the documentation some succinct information on how the detuning offset and light shift is computed (with references on where it comes from).

@HGSilveri HGSilveri requested a review from madagra November 25, 2022 17:37
Copy link
Collaborator

@madagra madagra left a comment

Choose a reason for hiding this comment

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

Thanks, it looks good to me.

@HGSilveri HGSilveri added this to the v0.8 Release milestone Nov 28, 2022
@HGSilveri HGSilveri merged commit c437beb into develop Nov 28, 2022
@HGSilveri HGSilveri deleted the eom-channel branch December 9, 2022 11:20
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