Change waveform memory estimation #931
Conversation
for more information, see https://pre-commit.ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 0.2 #931 +/- ##
=======================================
Coverage 45.09% 45.10%
=======================================
Files 70 70
Lines 6240 6241 +1
=======================================
+ Hits 2814 2815 +1
Misses 3426 3426
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| ( | ||
| (pulse.duration if not isinstance(pulse.envelope, Rectangular) else 1) | ||
| if isinstance(pulse, Pulse) | ||
| else 1 |
There was a problem hiding this comment.
I am guessing this case is for Delay or VirtualZ, so it could be
| else 1 | |
| else 0 |
?
There was a problem hiding this comment.
The case of VirtualZ is different, and might be actually a 0 (I left a TODO for that). But for Delay you actually need one number: the duration.
Thus, the Rectangular should be two numbers, duration and amplitude. However, the truth is that at this level they're more or less random, and 1 or 0 is practically the same, since you will consume your memory first with other pulses (or you will consume a different type of memory).
We should go more in the details of the various instruments memories, and we have #698 for that. For the time being, let's keep it just a little bit less random (avoid counting non-existing samples for rectangular pulses and delays), and refine them later on.
Retrieving the sampling_rate from somewhere would already be a macroscopic advancement. But not required as well (since it can be traded by a different limit in the Bounds instance).
This should be slightly more accurate than the former, since the rectangular pulses should all weight the same, not just the readout ones (and even readout pulses could require samples, if they are non-rectangular).
It is still incredibly rough for a lot of reasons, but the main one could even be that we're using the duration in ns, disregarding the sampling rate.