Closed jsdillon closed 8 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
790428f
) 97.17% compared to head (67d0c29
) 97.18%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Despite some misleading results from %memit, it appears that omnical's memory usage is the root of some of the problems we've been having with RTP.
This PR improves the memory efficiency of omnical. It does so by studiously avoiding copying data unnecessarily, as well as by updating dictionaries with for loops rather than dictionary comprehension, so that we don't have to make a whole new dictionary before deallocating the old one.
It also changes the default value of
wgt_func
fromlambda x: 1.
toNone
and adds special treatment of theNone
case so that it doesn't multiply by 1 and create new arrays.It has a negligible effect on performance (and actually might be a tiny bit faster). In my tests on the file_calibration notebook with
zen.2460280.48839.sum.uvh5
locally, runtime improved from 5.07 minutes to 4.63 minutes. It improves peak memory usage from 18.8 GB to 12.3 GB, although it should be noted that ~6.5 GB was the memory usage (mostly raw data) going into omnical.Here's my memray results before:
And after:
It's possible we can do even better if we use single precision, which I'll explore next.
This PR relies on https://github.com/HERA-Team/linsolve/pull/51