Suggestion for improvements and bugfixes

[CURTLab]

Description

I really like the careamics-napari but there are still some bug to make it usable. I had a look the code and created a fork to try and fix some bugs for multidimensional images.

Parts to improve:

• Add batch size to prediction
• Reshape the prediction to match the input axes
• Enable patch_Z_spin when enable_3D is checked
• Better handle the situation if the number of channels is not correct set in advanced settings

I would be happy to make a Pull Request. https://github.com/CURTLab/careamics-napari

Which part of the code?

• Add batch size to prediction:
  • src/careamics_napari/signals/prediction_signal.py
  • src/careamics_napari/widgets/prediction_widget.py
  • src/careamics_napari/workers/prediction_worker.py
• Reshape the prediction to match the input axes:
  • src/careamics_napari/training_plugin.py
• Enable patch_Z_spin when enable_3D is checked:
  • src/careamics_napari/widgets/training_configuration_widget.py
• Better handle the situation if the number of channels is not correct set in advanced settings:
  • src/careamics_napari/workers/training_worker.py

Examples

import napari
from careamics_napari import TrainPluginWrapper

viewer = napari.Viewer()
viewer.window.add_dock_widget(TrainPluginWrapper(viewer))
napari.run()

Potential solutions

• Add batch size to prediction:
  • Add a spinbox to the tiled prediction (1eea399)
• Reshape the prediction to match the input axes:
  • Add a _reshape_prediction function to training_plugin.py which handles the transformation (d4b2e90)
• Enable patch_Z_spin when enable_3D is checked:
  • Enable the spin box for z in the function _enable_3d_changed (cc6f8da)
• Better handle the situation if the number of channels is not correct set in advanced settings:
  • Add a try block around the function create_configuration. A better way would be to notify the user using napari.utils.notifications (dd6f590)

[jdeschamps]

Hi there,

Lovely!

Yes, there was big push to release the plugin for I2K, and there were plans to fix the obvious issues right after, but life caught up. I was planning to spend time fixing stuff over Christmas once the end of year craziness would calm down. Really appreciate you spending time on this.

Some thoughts following.

Batch size

Yes, totally useful! Currently, we didn't really see any improvement in speed with higher batch sizes. We think that there is an issue in Lightning creating overheads with batch_size>1. This was at the time of the tests and we have had no time to investigate further.

Absolutely mergeable directly!

Match input axes

Love this! S and T are a bit of an annoyance, but we could remember the original axe dimensions in the prediction signal object and apply the inverse mixing.

Mergeable right away!

Enable the 3D spin

Good catch! Mergeable right away as well.

The check for the configuration not being None is probably for mypy. We should investigate whether there is a point in having the configuration as Optional at all.

Originally, it was to be able to run the widget without string attached. But now it seems even the __main__ is creating the configuration.

Try catch clause

Agreed!

Regarding the napari notifications, there is a plan to make the code-base napari independent. Hence, why there are not many of these yet. Currently it is totally napari dependent, but you'll notive that it is done to a minimum level.

---

PRs are more than welcome!! Usually, we would ask for single PRs per topic. But everything I see is mergeable right away. So up to you, if you have multiple branches then open multiple PRs, otherwise a single one will do!

Thanks a lot!!

[CURTLab]

Hi,

I found a critical bug in the configuration of tiles for 3D images: de6b334

Also I further tested the reshape_prediction method which needed some fixes and moved the function to src/careamics_napari/utils/axes_utils.py.

I added tests for mutiple combinations of axes: tests/test_predictions.py
2c7e2e7

How should we proceed?

[jdeschamps]

Good catch!

Make a PR so I can have a look. Things are going to be slow over Christmas, but I'll try.

We usually use pytest for testing, although we did not spend the necessary time yet on writing tests for the plugin. So I'd rewrite your tests ultimately. But in a first step, I'm happy to merge it as in (I will need to have a closer look, but I'd do that in the context of a PR).

But the answer is PR, PR, PR! 😄

Thanks for spending the time!!