CellProfiler / CellProfiler-plugins

Community-contributed and experimental CellProfiler modules.
http://plugins.cellprofiler.org/
56 stars 67 forks source link

Update Cellpose model #178

Closed jenna-tomkinson closed 1 year ago

jenna-tomkinson commented 1 year ago

This PR is to update the runcellpose.py to call models using model.Cellpose instead of model.CellposeModel.

The main difference between the two, as found by @roshankern, is as follows:

Based on the latest documentation (https://cellpose.readthedocs.io/en/latest/models.html) that a model loaded with CellposeModel() does not estimate the ROI size if diameter is set to 0 -- only a model loaded from Cellpose() will estimate the cell diameter.

Under "Full Built-in Models"(https://cellpose.readthedocs.io/en/latest/models.html#full-built-in-models): These models can be loaded and used in the notebook with models.Cellpose(model_type='cyto') or in the command line with python -m cellpose --pretrained_model cyto. These models have a size model and 4 different training versions, each trained starting from 4 different random initial parameter sets. This means you can run with diameter=0 or --diameter 0 and the model can estimate the ROI size.

Under "Other Built-In Models"(https://cellpose.readthedocs.io/en/latest/models.html#other-built-in-models): These models do not have a size model and 4 different training versions. If the diameter is set to 0.0, then the model uses the default diam_mean for the diameter (30.0). These models can be loaded and used in the notebook with e.g. models.CellposeModel(model_type='tissuenet') or models.CellposeModel(model_type='LC2'), or in the command line with python -m cellpose --pretrained_model tissuenet.

jenna-tomkinson commented 1 year ago

Hi @bethac07 and @bdiazroh!

I was wondering if either of you could give this small PR a quick look! Only 4 lines of code changed but it definitely makes a big impact on the models for Cellpose.

Thank you!

bethac07 commented 1 year ago

Am I reading this incorrectly, or would this therefore make all but the 4 main models in Cellpose throw an error? We definitely do not want to do that. Can you confirm if that's the case?

If so, you would either need to add language that checks if it's a model that's appropriate to use that for, or just maintain the change in your own fork. Thanks!

jenna-tomkinson commented 1 year ago

Hi @bethac07,

Thank you so much for bringing that piece of documentation to light. When we tested with this file, we only used the cyto model so we would have never realized that. I just did a test with on of the non-main models and as expect, received an error.

I will close this PR and revert to the current version. As you said in your response, we will maintain the change in the project that it applies to.

Thank you again for going over this PR!