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

Flexible Sequence drawing #385

Merged
merged 11 commits into from
Jul 18, 2022
Merged

Flexible Sequence drawing #385

merged 11 commits into from
Jul 18, 2022

Conversation

HGSilveri
Copy link
Collaborator

  • Makes the detuning curve appear only when the detuning is being used (i.e. is not zero throughout the channel)
  • Adds the option to draw the phase in a curve (draw_phase_curve) in drawing methods
  • Simplifies the color scheme for programmed vs modulated waveforms by using a hatch on output waveforms instead.

Examples:

  • Bell state creation sequence, which uses only resonant pulses so the detuning curve is automatically omitted

image

  • The phase curves show up when draw_phase_curve=True. The detuning is still omitted if not used (as happens for the"raman_local"` channel)

image

To Do:

  • The phase curves are for now being extracted by the _seq_drawer.gather_data() function, as is still the case for the amplitude and detuning. As outlined in Sequence sampler feature extension #349, we would like these to come from the sampler instead.
  • There is some freedom in the moment the phase is changed that can be used to minimize the number of phase jumps and the effects of modulation. This should also be done centrally by the sampler.

Closes #332 .

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@HGSilveri HGSilveri requested a review from CdeTerra July 7, 2022 08:02
Copy link
Collaborator

@CdeTerra CdeTerra left a comment

Choose a reason for hiding this comment

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

Some general style remarks, to be discussed and that could be applied later.

pulser-core/pulser/_seq_drawer.py Outdated Show resolved Hide resolved
pulser-core/pulser/_seq_drawer.py Outdated Show resolved Hide resolved
pulser-core/pulser/_seq_drawer.py Outdated Show resolved Hide resolved
pulser-core/pulser/_seq_drawer.py Outdated Show resolved Hide resolved
@HGSilveri
Copy link
Collaborator Author

@CdeTerra I agree with your remarks in general, I admit I tend to be more careless with functions for plotting because I don't expect people to read their contents often, but it's a bad habit. I'll give your suggestions a shot before merging, if I find they're not so hard to implement I'll still include them here.

@HGSilveri HGSilveri requested a review from CdeTerra July 15, 2022 16:25
Copy link
Collaborator

@CdeTerra CdeTerra left a comment

Choose a reason for hiding this comment

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

Good changes! Small new comments, but approving in general.

pulser-core/pulser/_seq_drawer.py Outdated Show resolved Hide resolved
pulser-core/pulser/_seq_drawer.py Outdated Show resolved Hide resolved
pulser-core/pulser/_seq_drawer.py Outdated Show resolved Hide resolved
@HGSilveri HGSilveri merged commit 58f5b0c into develop Jul 18, 2022
@HGSilveri HGSilveri deleted the flexible-seq-drawer branch July 18, 2022 09:22
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.

More flexible sequence drawing
2 participants