Correction runners as friends
Status
Done with what I had in mind for now.
Sorry that the diff is so big.
At least, looking at git diff --numstat master
, the individual too long files are a bit more balanced; some of the complexity of BaseCorrection
is now kept in BaseKernelRunner
and PreviewFriend
.
Also, detector-specific correction classes are much simpler.
Will do some more testing and figure out how to review.
Goal
Simplify interaction between correction device and kernel runner. To this end, must make the kernel runner smarter and more responsible for handling its relevant subschemata. Some plans:
- Device passes most correction schema (changes) along to runner which has
reconfigure
which looks at hash (before: device figures out everything, uses loads of special methods to configure runner)- This, in particular, means that the "correction flags" (in the end passed to correction kernel) live
- Device gets simpler interaction for running corrections and generating previews
- Related to flags, now the kernel runner must itself figure out what to do
- Before: host device tells it to load, correct, do previews, maybe correct again etc.
- Medium-long term plan (after this MR): should allow concurrent processing, meaning stateful "load this, now process whatever you've loaded" is out
- Other things like specialized
_load_constant_to_runner
goes away because each runner has uniformload_constant
interface
Input handler flow
One of the big changes is the flow of the input handler.
This section explains a solution to the issues I was pondering earlier (left as quote below).
Before, BaseCorrection
would have the main input handler which would extract (most, though not always all) relevant data from the hash and handle (most) quirks we tend to encounter before handing over to subclass-specific process_data
.
However, these different process_data
implementations had a lot of overlap.
Two obvious options: keep them separate, encapsulating all the things they do in small reusable functions or unify them and provide hooks / utility functions to fill in per subclass.
Currently trying out the latter:
- The input handler (now
ON_DATA
again*) is only inBaseCorrection
- It uses
_get_data_from_hash
(may be subclass-specific) to get all the relevant data out of the hash - It asks the kernel runner about
expected_input_shape
(needed to fix up wrong axis order from DAQ) - It asks the kernel runner about
expected_output_shape
(needed to prepare the next shmem output slot) - It then passes the extracted data to the kernel runner's
correct
method
And similarly, the BaseKernelRunner
hosts the overarching correct
method, letting subclass define certain parts:
-
correct
inBaseKernelRunner
gets all the data from_get_data_from_hash
- It uses
_make_output_buffers
(may be subclass-specific) to set up some buffers the kernel will write to - It uses the subclass-specific
_correct
to actually do the correction- This
_correct
gets all the data and the output buffers, so most of_get_data_from_hash
and_make_output_buffers
goes here
- This
- Then, previews are computed, with
_preview_data_views
(may be subclass-specific) providing the views of the relevantndarray
s to preview- This gets the image data, any additional data from
_get_data_from_hash
, and the output buffers from_make_output_buffers
and can return arbitrary list of buffer views - Each buffer view returned should match a preview output of the device; this allow preview computation to just be applying the same preview reduction function on each view and write it out
- This gets the image data, any additional data from
Although an improvement to the previous version, I'm not pleased that the different parts are still so tightly coupled. Not sure how to avoid this.
*A long time ago, we switched from ON_DATA
to ON_INPUT
, partly because - in case we'd get irrelevant sources coming in - the latter would allow us to not read content of certain input hashes unless relevant to us.
ON_DATA
is simpler, however, and as I understand from Gero's description of the surrounding C++ code changes, we don't decide whether or not to read the input any longer (the C++ code reads hashes and just gives them to us), so the potential performance edge of ON_INPUT
should be gone.
And we don't need trainmatched data, so it's not important to line up hashes on same input event.
Follow-up work
I've removed the frame filter functionality for now for two reasons
- It complicates
BaseCorrection
, configuration, and shape handling - It slices data out before it gets previewable; could be an issue for detector monitoring
- OTOH, the operator may want the
nanmean
of only the relevant / lit frames; but frame filter cannot accommodate both use cases
- OTOH, the operator may want the
I know that MID tends to use the frame filter, so I'd just add a trivial frame selection kernel with the same logic.
Deployment notes
Having restructured the preview output infrastructure anyway, I also tied it to more node-local subschemata.
So now each preview output can have its own settings about how to reduce (it was kind of silly to, say, look at mean of corrected and then also have to get mean of gain map).
With a node for each output, preview.[preview_name]
, I figured we could move the outputs there, so preview.[preview_name].output
.
This means that managers all over will need an update to the preview layer mapping* and stored scenes referencing previews need updating.
*Technically, the manager can now figure out the preview outputs easily by looking (in the schema) at the keys under preview
.
Indeed, the correction_device_overview
scene does exactly this to generate per-device preview scene links.