Rework detector selection for preprocess loading #1116
Conversation
| logger.info("Restricting detectors on all processes") | ||
| for process in pipe[:]: | ||
| process.select(meta) |
There was a problem hiding this comment.
I think since you fixed this here, you don't need to run the select step in load_and_preprocess so on line 347 you should be able to run pipe.run with select=False and if other functions call load_preprocess_det_select before pipe.run then the select=False should be used there as well. But we should test that it doesn't break other things.
It would be awesome if load_and_preprocess_det_select could collect all of the restriction flags and union them and do one call to restrict as there is quite an overhead to each call to restrict (hence my suggestion to not call it again in pipe.run).
There was a problem hiding this comment.
I added select=False for both single and multilayer loading pipeline.run() calls. Tested in both cases and confirm they begin with the correct number of detectors and loading runs to completion.
It would be awesome if
load_and_preprocess_det_selectcould collect all of the restriction flags and union them and do one call to restrict as there is quite an overhead to each call to restrict (hence my suggestion to not call it again in pipe.run).
For this, we could add an in_place option to the select functions which would apply flags if True and return the flags (or some new axis manager) instead if False. I think that would be the most general way of handling this.
| if in_place: | ||
| meta.restrict("dets", meta.dets.vals[keep]) | ||
| return meta | ||
| else: | ||
| return keep |
There was a problem hiding this comment.
since this 5 line block is copied in every select step should we make a method of the Pipeline class called restrict or cut that takes in meta, in_place, and keep and then make these 5 lines: return self.cut(meta, keep, in_place)?
sotodlib/preprocess/processes.py
Outdated
| if in_place: | ||
| meta.restrict("dets", meta.dets.vals[keep]) | ||
| source_flags.restrict("dets", source_flags.dets.vals[keep]) | ||
| else: | ||
| keep_all &= keep | ||
| if in_place: | ||
| return meta | ||
| else: | ||
| return keep_all |
There was a problem hiding this comment.
Why is this one different than the rest? This breaks my generalization :(
|
After some offline discussion we decided to just merge this without moving to a single function since there are differences in the handling between processing functions and other open PRs are adjusting them. Added one more change to the noisy subscan flags which didn't have the
Since I think it should be guaranteed to exist and we don't do this check elsewhere. This makes sure it always returns either |
|
Okay after the two latest changes I ran all the way through and reloaded with the iso-v2 configs and confirmed that it loads with the same number of detectors that survive to the end of the initial run. |
This branch changes the way detectors are restricted when loading from either a single layer or multilayer preprocessing run. Detectors are now restricted to the number remaining at the end of the initial preprocess run.