DM-41884: Use more memory efficient nanquantile for large pixel spread.#71
DM-41884: Use more memory efficient nanquantile for large pixel spread.#71
Conversation
mfisherlevine
left a comment
There was a problem hiding this comment.
Sorry this took so long to get to. So, I think the code changes look great, thanks. My only question is whether 131072 is the right point to switch over to the other method or not. I tried doing a bit of quick profiling, but couldn't reproduce (or rather, catch) the huge memory usage we were seeing before with the notebook tools I was using. Ideally, we'd have some measure of where the memory usage gets silly, and how the runtimes compare, so that we could make some more informed choices, but I realise that's quite a bit of scope creep to drop on the ticket in the review.
So, I think my question is: given we have 18 bit ADCs, gains close to 1, and don't expect to go wildly negative, 2^18=262144 might be a better choice than the (presumably?) equally arbitrary 2^17 we have now, right? I'm just thinking that we'll routinely see numbers down to ~zero, and above 131072, meaning we'll tend to be in the switchover case most of the time, and we'd probably regain the performance boosts if we upped that, not to mention that we'd then be in a situation where the normal behaviour would be consistent, rather than routinely switching between the two modes based on the image values, and would only switch over in the case of pathological images.
So, assuming this wasn't done for deep reasons (like you having profiled or worked out where the other mode gets sad), I think we should up it buy a factor of two (or actually maybe a little more - presumably there's nothing too magic about powers of two here, so maybe 300k would be better so we can handle ADC_MAX * GAIN). And actually, given that, just making a logger at the point and logging a warning that the image contains unusual values and we're changing the binning mode, would, I think, be really nice too.
2dba354 to
f80d39a
Compare
No description provided.