feat(preprocess): add tiling CLI support #207
Conversation
hollandjg
commented
Dec 19, 2024
•
hollandjg
commented
- Add new preprocessing function which uses the tiling option
- Add "LopezTiling" as a PREPROCESSING option in the workflow.
…/WilhelmusLab/ice-floe-tracker-pipeline into add-tiling-preprocessing-support
hollandjg
requested review from
mirestrepo,
danielmwatkins,
tdivoll and
cpaniaguam
There was a problem hiding this comment.
Thanks for this! A few things to consider:
- Add support for tiling by tile size
- Extend user-facing documentation about the tiling workflow (histogram equalization, binarization, ice-labeling)
- Move tiling workflow parameters to a configuration file
- Use
-as separator for cli long options
| @info "Set structuring elements" | ||
| # This isn't tunable in the underlying code right now, | ||
| # so just use the defaults | ||
| structuring_elements = IceFloeTracker.structuring_elements |
There was a problem hiding this comment.
Not sure if this would need to be flexible as well. @danielmwatkins, what do you think?
There was a problem hiding this comment.
Well, we know that the size and characteristics of the segmentation results are going to be affected by the shapes and sizes of the structuring elements. What might be necessary eventually, since there are so many options, is for my initial work on the calibration to identify the parameters that the results are really sensitive to so that those are the ones accessible from the command line.
IFTPipeline.jl/src/cli.jl
Outdated
| @@ -493,6 +680,10 @@ function main() | |||
| help = "Preprocess truecolor/falsecolor images" | |||
| action = :command | |||
|
|
|||
| "preprocess_tiling_single" | |||
| help = "Preprocess truecolor/falsecolor images with tile-based histogram equalization" | |||
There was a problem hiding this comment.
The tiling workflow does more than histogram equalization. It also does binarization and ice-water discrimination (ice labeling).
There was a problem hiding this comment.
I think that's implied by the "preprocess". I think that the difference between the tiling and the non-tiling preprocessing is the tiling alone. Is that correct? If so, it makes sense that that's all that's included in the string.
There was a problem hiding this comment.
The tiling is indeed a significant aspect of the workflow, but there are other important operations beyond histogram equalization within the tiled workflows. To better reflect the full scope of the processing, it would be clearer to either mention these additional operations or reword the line to exclude 'histogram equalization'.
There was a problem hiding this comment.
Does this cover it?
"preprocess_single"
help = "Label ice in a single set of truecolor & falsecolor images"
...
"preprocess_tiling_single"
help = "Label ice in a single set of truecolor & falsecolor images, using histogram equalization"
There was a problem hiding this comment.
I was thinking something more like "Label ice in a single set of truecolor & falsecolor images using tiled-based processing".
There was a problem hiding this comment.
Great, thanks. Updated. What do you think?
Co-authored-by: Carlos Paniagua <[email protected]>
…/WilhelmusLab/ice-floe-tracker-pipeline into add-tiling-preprocessing-support
There was a problem hiding this comment.
@cpaniaguam I think you're looking at an earlier commit. Here's the current one.
| required = false | ||
|
|
||
|
|
||
| # Gamma parameters |
There was a problem hiding this comment.
| # Gamma parameters | |
| # Gamma adjusting parameters |
