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

The computed duration of erfsquare waveform templates is incorrect #405

Open
antalsz opened this issue Sep 20, 2024 · 2 comments · May be fixed by #406
Open

The computed duration of erfsquare waveform templates is incorrect #405

antalsz opened this issue Sep 20, 2024 · 2 comments · May be fixed by #406

Comments

@antalsz
Copy link
Contributor

antalsz commented Sep 20, 2024

ScheduledBasicBlock::get_waveform_duration_seconds assumes that the duration of all built-in waveforms is equal to their duration parameter. This isn't true for erfsquare, though; the duration is just that of the main pulse, and the padleft and padright parameters specify silent padding on either side, which contributes to the total duration.

To see this, consider the following program:

DEFFRAME 0 "a":
    SAMPLE-RATE: 1e9
PULSE 0 "a" erfsquare(duration: 1.0, padleft: 0.2, padright: 0.3)
PULSE 0 "a" erfsquare(duration: 0.1, padleft: 0.7, padright: 0.7)
PULSE 0 "a" erfsquare(duration: 0.5, padleft: 0.6, padright: 0.4)
FENCE

If provided as a test case to program::schedule::tests::schedule_seconds, it will produce instruction timings of [0.0, 1.0, 1.1, 1.6], when it should instead produce [0.0, 1.5, 3.0, 4.5].

@erichulburd
Copy link
Collaborator

Is the solution in quil-rs to add total_duration to the WaveformTemplate trait?

@antalsz
Copy link
Contributor Author

antalsz commented Sep 20, 2024

@erichulburd That's part of a more in-depth solution, but it requires figuring out how WaveformTemplate integrates into quil-rs; as it stands, it's exported but never used internally. This means that deciding on how WaveformTemplate should be used is a bigger change, and I'd like to fix this specific issue before we resolve that.

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 a pull request may close this issue.

2 participants