Unify preprocessing functions #1027
Conversation
There was a problem hiding this comment.
Thanks @mmccrackan, some comments on things that are also done in #1014 which has not been merged yet. How should we rectify this? @msilvafe
| @@ -12,35 +12,6 @@ | |||
|
|
|||
| logger = sp_util.init_logger("preprocess") | |||
|
|
|||
| def _get_preprocess_context(configs, context=None): | |||
There was a problem hiding this comment.
This removal is also done in #1014
| @@ -56,265 +57,6 @@ def dummy_preproc(obs_id, group_list, logger, | |||
| if run_parallel: | |||
| return error, dest_file, outputs | |||
|
|
|||
| def _get_preprocess_context(configs, context=None): | |||
There was a problem hiding this comment.
This removal is also done in #1014
| ) | ||
| return configs, context | ||
|
|
||
| def _get_groups(obs_id, configs, context): |
There was a problem hiding this comment.
This removal is also done in #1014
It looks like the only major difference is that I didn't add the get_obslist function that you wrote. I can add that here since it looks useful for the multilayer_preprocessing. Since this pull request includes the preprocess_util function, I guess we can say that this branch supersedes #1014? TODOs: Finally, we will attempt to merge preprocess_tod, preprocess_obs, and multilayer_preprocess_tod into a single script. |
|
Just to comment on the latest pushes, these re-arrange the shared functions such that neither Some of the functions were used in This approach should mostly maintain the hierarchy of |
sotodlib/mapmaking/demod_mapmaker.py
Outdated
| @@ -523,6 +523,7 @@ def make_demod_map(context, obslist, noise_model, info, | |||
| List of outputs from preprocess database. To be used in cleanup_mandb. | |||
| """ | |||
| from .. import site_pipeline | |||
There was a problem hiding this comment.
This import here is only for the logger. @mhasself does not want us to import the site pipeline stuff as if it were a library. On the other hand, this logger is needed here because passing a logger from the script side to this function won't work with multiprocessing, as it won't print messages. Hence on this function, we have to create a new logger object to be able to print messages (this is how @msilvafe described it to me). Is there a workaround to this? Any suggestions? Maybe the logger stuff should also be moved to the library side.
There was a problem hiding this comment.
We've copied the logger stuff into preprocess/preprocess_util.py so that the preprocessing module also has its own logger available. The mapmaking module could use that one or it could copy the logger functions into its own util file of some kind maybe.
There was a problem hiding this comment.
ok, perfect. Can you change the logger to be created from preprocess_util then, please?
This branch moves around some of the preprocessing functions into a common
preprocessing/preprocess_util.pyfile. The ones moved specifically are:load_preprocess_det_selectload_preprocess_tod/load_preprocess_obs(merged intoload_and_preprocess)preproc_or_load_group(to address preproc_or_load_group in preprocess submodule #1025)cleanup_mandbSome of these aren't shared among multiple files yet but they will be when #1026 is merged. Note that the inputs of
load_and_preprocessare slightly modified fromload_preprocess_tod/load_preprocess_obs. It now acceptsno_signalwhich defaults toNoneand thedetsandmetainputs not inload_preprocess_obsare now added. The default config names were removed, though we could re-add them based on the value ofno_signalperhaps.Some additional files have also been moved into
site_pipeline/util.py:swap_archiveget_preprocess_db(edited to accept an optional logger as input, creates one if not given)