core-bioimage-io-python icon indicating copy to clipboard operation
core-bioimage-io-python copied to clipboard

bug when `add_deepimagej_config=True` in `build_model`

Open esgomezm opened this issue 8 months ago • 5 comments

Halo!

When building a model and adding the deepImageJ config with the IJ macros, if the pre and post-processing need to use the same pipeline (e.g., scale range), there is the issue of downloading just one macro rather than two different versions of it (pre+postprocessing).

For example, the following code will download the scale_linear.ijmfile twice (one for the preprocessing and one for the postprocessing) but with the same name and in the deepImageJ config, both pre and post-processing will point to the same ij macro file scale_linear.ijm but with the wrong parameters. Could it be possible to rename the downloaded macros accordingly to something like pre_scale_linear.ijm and post_scale_linear.ijm and include as such in the deepImageJ config?

# Preprocessing steps
bmz_preprpocess = [[{"name": "scale_range", "kwargs": {"min_percentile": 2.,
                                                       "max_percentile": 99.9,
                                                       "mode": "per_sample",
                                                       "axes": "xy"}},
                    {"name": "scale_linear", "kwargs": {"gain": 2,
                                                        "offset": -1,
                                                        "axes": "xy"}}]]
# Post processing steps
bmz_postprocess = [[{"name": "scale_linear", "kwargs": {"gain": 0.5,
                                                        "offset": 0.5,
                                                        "axes": "xy"}}]]

# Input & output specs
# ---------------------------------------
kwargs = dict(
  input_names=["input"],
  input_axes=["bcyx"],
  pixel_sizes=[pixel_size],
  preprocessing = bmz_preprpocess)

output_spec = dict(
  output_names=["output"],
  output_axes=["bcyx"],
  postprocessing=bmz_postprocess, 
  output_reference=["input"],
  output_scale=[4*[1]], 
  output_offset=[4*[0]]
)
kwargs.update(output_spec)
 
...

  build_model( ..., 
      add_deepimagej_config=True,
      **kwargs)

Thank you!

esgomezm avatar Dec 12 '23 10:12 esgomezm

Implementing that would be possible (probably rather as part of the save_bioimageio_package and its related functions). But allow me to raise the question: When will the DeepImageJ plugin be able to create the required macros on their own from the model description? Including them explicitly in the package was always meant as a temporary solution...

FynnBe avatar Dec 14 '23 01:12 FynnBe

Hi hi

Implementing that would be possible (probably rather as part of the save_bioimageio_package and its related functions).

I can give it a try if you point me where this is done. I needed it for some zerocost notebook conversion that I just finished.

But allow me to raise the question: When will the DeepImageJ plugin be able to create the required macros on their own from the model description? Including them explicitly in the package was always meant as a temporary solution...

Yes, you're right. JDLL has the functions implemented. I raised it to @arrmunoz already. Tagging @cfusterbarcelo also to keep track of this.

esgomezm avatar Dec 14 '23 08:12 esgomezm

If only a few models are affected I suggest to fix the exported models manually. If the next spec library release implements this as well, we can use f"{tensor_id}_{macro_file_name}" as final file name to avoid file name collisions. I have a draft for that already, but ideally we won't need a special deepimagej section under config soon and implement the required logic in DeepImageJ instead.

FynnBe avatar Dec 15 '23 21:12 FynnBe

Hi Fynn,

Thank you! As long as JDLL is updated with the specs, you are completely right and that's how it should be. So, do you want me to test the changes in the PR?

esgomezm avatar Jan 11 '24 15:01 esgomezm

well, the above PR isn't ready to be merged, see https://github.com/bioimage-io/spec-bioimage-io/pull/546#discussion_r1449450503

FynnBe avatar Jan 11 '24 22:01 FynnBe