-
Notifications
You must be signed in to change notification settings - Fork 22
Improved Full Detector Response storage and conversion #364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
jdbuhler
wants to merge
31
commits into
cositools:develop
Choose a base branch
from
McKelvey-Engineering-CSE:new_response_format
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Improved Full Detector Response storage and conversion #364
jdbuhler
wants to merge
31
commits into
cositools:develop
from
McKelvey-Engineering-CSE:new_response_format
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
into a new RspConverter class * change on-disk response format to split out counts from effective area, and use low-overhead compression on counts * allow counts to be stored in integer types smaller than 32 bits; provide auto-detection of the best size as an option * Rework .rsp conversion code to use less memory * Remove outdated support for sparse .rsp and broken code for file-based normalization * Enable axes of response to be read and written via the Axes object instead of replicating its functionality. Keep textual descriptions of axes in a separate group for pretty-printing. * Allow each Healpix axis to have its own nside; all-sky axes use nside=1 * capture and store header fields other than the axes and the size of counts in the .h5 file for future reference
tables stored at class scope. * Filter axes to be used for output HDF5 file as they are being read from .rsp, rather than arbitrarily removing the last couple of axes afterwards.
…deconvolution * make default for response construction to infer the element size from the data, rather than guess a too-large size
the content of the response * fix __array__ method of FullDetectorResponse * use new test full detector and polarization responses, and update test suite to expect the outputs that occur when they are used (work in progress)
* remove backup file that snuck into docs
* add conversion to Histogram to FullDetectorResponse API, rather than open-coding it in image_deconvolution/dataIF
not relevant to the output being tested and occasionally causes crashes
…sponse * add unit tests for response conversion between .rsp and .h5 * fix typo in FullDetectorResponse.to_histogram()
* remove old commented-out code
* fix typo in open()
* to further avoid confusion between response versions, rename CONTENTS dataset to COUNTS, since it is now raw counts * FullDetectorResponse.open() should support only HDF5, as we cannot use an .rsp file live, it is not reasonable to convert a usefully large (o3 or above) .rsp.gz file "on the fly", and we have a separate converter class now.
…nverting the same .rsp to .h5 twice yields identical byte streams. * Do order tracking of headers ourselves instead of relying on HDF5 to do it.
…er than "DC2/DC3", with new checksums. Shorten the names while we're at it. * Remove vestiges of miniDC2 from image deconvolution notebooks * make sure setup for cosipy installs hdf5plugin package
existence of COSI-SMEX/develop tree on wasabi
* update .h5s for test full detector responses to newest on-disk format
eff_area, which controls the type returned by __getitem__, etc. This lets us have a float32 response instead of float64 if desired.
Empirically, we get slightly better compression and notably faster decompression without the extra shuffle
entire response in memory in order to do F-to-C order transposition. Instead, we do the transposition one chunk at a time, which roughly halves memory usage for large responses.
not a general Histogram. Rename the relevant function from to_histogram() to to_dr() and update its return type accordingly
* Add get_pixel() method to implement __getitem__, but with the possibility of specifying a weight that we can apply to eff_area rather than to the much larger counts matrix for greater efficiency. Use this method in get_psr() and friends.
with new "quiet" option to RspConverter class - response test cases now use get_pixel() rather than __getitem__ for most tests
and use it in RspConverter when converting back to .rsp
Codecov ReportAttention: Patch coverage is
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
so we can exercise it with smaller sizes * fix file exists check
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR overhauls the on-disk and in-memory format of the FullDetectorResponse. The principal goals are to
The biggest change is that the raw CDS ring counts are stored as small integers on disk, separately from the effective area multipliers. This change greatly improves compressibility and reduces peak memory during rsp->h5 conversion. As a bonus, it allows more efficient weighted averaging of slices at runtime and dynamic selection of 32- vs 64-bit PSRs.
Conversion has been separated from the FullDetectorResponse class into its own RspConverter class, which handles both rsp->h5 and h5->rsp.
After consultation with Israel et al., these changes remove support for the following deprecated formats and features:
The new on-disk response format is not backwards or forwards compatible with the previous format. For this reason, I've established a new directory COSI-SMEX/develop in the public cosipy bucket on wasabi, put the new response files there, and updated the tutorials to use them instead of the ones from DC2/DC3. It is anticipated that DC4 will use the new format, while DC3 will remain with the current one.
The patch introduces a dependency on the hdf5plugin package, a well-supported companion to h5py that implements the BitShuffle compression algorithm used by the new response format.
Supporting performance numbers
Conversion time and memory
DC2 O3 continuum response: < 4 minutes, 4 GB
DC3 O3 polarization response: < 6 minutes, 6.33 GB
O4 version of continuum response: < 90 minutes, 52.7 GB
Testing was on a 2.3 GHz Intel Xeon Gold 5118 server with 192 GB of RAM and SSD storage; only one core was used.
(The first two used to take hours, while the last took overnight and needed at least half a terabyte of RAM)
HDF5 response file sizes on disk
DC2 O3 continuum response: 592.76 MB (.rsp.gz is 841.58 MB)
DC3 O3 polarization response: 795.76 MB (.rsp.gz is 954.58 MB )
O4 version of continuum response: 4187.05 MB (.rsp.gz is 5186.87 MB)
These file sizes are < 1/10 the size of the previous uncompressed HDF5 responses for O3, and more like 1/50 for O4
response, assuming (as was the case for the file I received) the latter is stored in float32 precision.
Read times
The following are average times to read one slice from the NuLambda axis (i.e., rsp[i]). Testing was on the same machine used for the conversion tests.
DC2 O3 continuum response: 4.5 ms (old), 6.4 ms (new)
DC3 O3 polarization response: 8.8 ms (old), 11 ms (new)
O4 version of continuum response: 350 ms (old), 46 ms (new)
"Old" is the previous response format, while "new" is the format in this PR. Note that for large enough responses, such as the O4 continuum response, it's faster to read less data from disk and decompress than to read the data uncompressed.
By way of comparison, compressing the DC2 O3 continuum response with HDF5's gzip internal compression yielded a substantially larger file (889.77 MB) and read times of 21 ms, > 3x longer than with the BitShuffle compression used in this patch.