MazinLab / MKIDGen3

GNU General Public License v3.0
8 stars 3 forks source link

capture_iq computes the time required for a capture job incorrectly #15

Open ld-cd opened 9 months ago

ld-cd commented 9 months ago

Capturing 2^24 samples from the raw IQ tap should take (2**24) / 2e6 seconds (8.388 seconds), but it is computed as taking 8.0 seconds by capture here:

https://github.com/MazinLab/MKIDGen3/blob/powersweeping/mkidgen3/drivers/capture.py#L363

ld-cd commented 9 months ago

We likely have not hit this yet but it is not impossible that we will with improvements to the memory subsystem

JennySmith888 commented 9 months ago

@baileyji Can you explain these numbers? datarate_mbps = 32 * 512 * n_groups / 256 Why not do datarate_mbps = [datavolume_mb] * [2 MHz]?

For Aled's test: data_vol = 2^24 samples with 2 groups is (2^24 samples)(16 groups)(4 bytes / sample) = 1024 MiB cap_time = 2^24 samples / (2e6 MHz)= 8.389 sec datarate_mbps =1024 MiB / 8.389 sec

Also mbps is very confusing bits / bytes so maybe let's use _by or just stick with things in bits?

baileyji commented 8 months ago
        # NB this ignores the final bit from a non-128 multiple
        datavolume_mb = capture_bytes / 1024 ** 2
        datarate_mbps = 32 * 512 * n_groups / 256
        captime = datavolume_mb / datarate_mbps

All three of those lines are for logging only and were written as ballpark numbers. Then in the shuffle with interrupts and that mess the captime wound up being used for a wait. Broadly one can compute it exactly for all cases, pad it, or sort out the remaining issues (if any) to actually use interrupts in all scenarios.

I wrote it this way

        datavolume_mb = capture_bytes / 1024 ** 2
        datarate_mbps = 32 * 512 * n_groups / 256

to come at the two from two different ends and see if I got similar numbers. Bytes being captured is explicit (though that was long before there wasn't a one-to-one correspondence given captures of subset channels). The datarate there is, as I recall, 32 (bytes per IQ group) 512 MHz pipeline rate (number of groups captured / number of possible groups), in essence the pipeline datarate * the fraction of the pipeline data being grabbed. But this clearly doesn't consider pre/post lowpass.

That more explicit math looks good, though I think it still might have a pre/post lowpass gotcha. Also I'd pull the 2e6 from system_parameters

Yeah, mbps could be mibps, or just move everything to bytes and use format_bytes since that exists and is so handy!

ld-cd commented 8 months ago

Wait should probably busy wait on the r_complete bit (with a reasonable timeout) if not using interrupts and check for an error, I am planning on posting a patch for this when I get a chance to come back around and work on this, probably before the next cooldown when I'm next planning on using this functionality next

baileyji commented 8 months ago

That should work nicely, though I have a vague memory of early on attempting to use that bit and finding it did not behave as I expected, leading me to abandon that approach. This would have been a few years ago.