kreshuklab / plant-seg

A tool for cell instance aware segmentation in densely packed 3D volumetric images
https://kreshuklab.github.io/plant-seg/
MIT License
89 stars 30 forks source link

BioImage.IO Core: Improve interface for PlantSeg models #248

Closed qin-yu closed 1 month ago

qin-yu commented 1 month ago

Changes

This PR improves #247

Before this PR

image

lorenzocerrone commented 1 month ago

Looks very nice! :) It's much cleaner.

Since we are discussing this widget, I have two more ideas/suggestions:

qin-yu commented 1 month ago
  • Is there ever a need to manually adjust the patch_halo for a standard user? To me, the default always worked the best. Maybe we should also remove it?

I originally planned to implement automatic halo sizing when I noticed that the halo size was set to a constant value. However, I shifted focus to correct the halo implementation in PlantSeg and pytorch-3dunet, and the initial plan was set aside. I’ll proceed based on this paper: https://nvlpubs.nist.gov/nistpubs/jres/126/jres.126.009.pdf. My belief is that group norm and wrong halo implementation (and perhaps constant halo size) worked in previous settings because the images were always similar. Now I trained new official models with batch norm that reduce hallucination, fixed halo padding that reduce tiling artefact (it's not too wrong to claim we "removed tiling artefact"). The next step is to automatically set a minimal halo size based on the architecture so that the prediction is as exact as theoretically possible and completely removes tiling artefact.

In the end, yes we remove the user-input halo, but do not use a constant. What do you think?

  • The Single Patch checkbox may be cryptic. What could be a better alternative? Maybe "Single Patch (Lower Memory Usage)"

Thanks for the advice, I'll fix it tomorrow!

lorenzocerrone commented 1 month ago

Cool paper, a great work retraining the moodels! :) I like the idea of havig the perfect halo size calculated from the architecture. My point is just that exposing the halo parameter to the users is probably more confusing than helpful.

qin-yu commented 1 month ago
  • The Single Patch checkbox may be cryptic. What could be a better alternative? Maybe "Single Patch (Lower Memory Usage)"

Hey Lorenzo, I don't actually understand the significance of having "Single Patch". If the model runs on host then PlantSeg finds batch size 1, otherwise finds the best batch size. I guess we can remove it and always find the best batch size for users?

But I made the changes you requested.

Update

There is one use case though: when someone has only one graphics card and plan to run PlantSeg for many images while using the same card. Yes, let's keep it.

lorenzocerrone commented 1 month ago
  • The Single Patch checkbox may be cryptic. What could be a better alternative? Maybe "Single Patch (Lower Memory Usage)"

Hey Lorenzo, I don't actually understand the significance of having "Single Patch". If the model runs on host then PlantSeg finds batch size 1, otherwise finds the best batch size. I guess we can remove it and always find the best batch size for users?

But I made the changes you requested.

Update

There is one use case though: when someone has only one graphics card and plan to run PlantSeg for many images while using the same card. Yes, let's keep it.

Yeah, that's exactly the use case. In general, if you have a single graphic card, it's nicer to keep a bit of Slack for other apps.