fix!: correctly compute duration for erfsquare waveform templates

[antalsz]

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

This PR updates the computation of the duration to include the padding. This is a breaking change, because now scheduling a program with an erfsquare waveform requires that the erfsquare waveform have these extra parameters and that they be constants. Indeed, some of our very own tests were broken by this.

There is also some inconsistency in names, which this PR papers over by accepting both:

• The Quil spec calls the waveform erfsquare, but our test cases call it erf_square
• The Quil spec calls the padding parameters padleft and padright, but our ErfSquare type calls them pad_left and pad_right.

As implemented, any mix of the underscoreless and underscoreful names can be used.

Closes #405.

[github-actions]

PR Preview Action v1.4.8
🚀 Deployed preview to https://rigetti.github.io/quil-rs/pr-preview/pr-406/
on branch quil-py-docs at 2024-09-25 20:13 UTC

[MarquessV]

Curious what others think, but I think this is okay because it adds a fix without adding tech debt that gets in the way of a more holistic future fix. I do wonder if we should rip the band-aid off and rally around either erfsquare or erf_square (and pad_x vs padx). Though, I don't know how disruptive that will end up being to users.

[kalzoo]

Good start, but I'd propose two changes:

1. The Quil spec can be "wrong." In fact, the only public-facing implementation is ours, and was written by some of the same people writing the spec. If erf_square and pad_* are how the template has been invoked for 4 years now, then we should just correct the spec, it was likely an oversight.
2. In my view it is tech debt to special-case the erf_square waveform like this. Just look for pad_left and pad_right on all waveforms and add those to the duration. Using duration naively and assuming it's in seconds and does not include the padding is already a short cut; IMO we don't make the shortcut any worse by making assumptions about duration-adjacent parameters. Further, this could be added to the Quil spec such that pad_* are supported for all of today's templates.

The "most correct" fix, I think, is to actually compute every waveform's IQs and then count samples. However, that is more computationally expensive and thus not necessarily superior to this approach. So let me propose a middle ground: unit tests which assert, for some varied parameters in all supported templates, that this duration + pad_* heuristic is identical to the actual length of the samples. So users can rely on that without having to perform the computation themselves.

[antalsz]

@kalzoo: I agree with your changes, but/and I think that the unit tests belong in a separate PR; to write them correctly, we have to handle parsing WaveformTemplates from WaveformInvocations, machinery that doesn't exist in quil-rs today. I've made the changes to commit to _ in the pad_* names and to always check for pad_left + pad_right.

[antalsz]

@kalzoo Sorry, I claimed to have pushed changes yesterday but it looks like they didn't actually go through. They went through now