-
Notifications
You must be signed in to change notification settings - Fork 196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cellprofiler app + plugin #1724
Cellprofiler app + plugin #1724
Conversation
Signed-off-by: binliu <[email protected]>
for more information, see https://pre-commit.ci
looking forward to this! |
Signed-off-by: binliu <[email protected]>
Signed-off-by: binliu <[email protected]>
for more information, see https://pre-commit.ci
Signed-off-by: binliu <[email protected]>
…iunls/MONAILabel into add-vsita2d-to-cellprofiler
for more information, see https://pre-commit.ci
Signed-off-by: binliu <[email protected]>
…iunls/MONAILabel into add-vsita2d-to-cellprofiler
Signed-off-by: binliu <[email protected]>
Signed-off-by: binliu <[email protected]>
Signed-off-by: Bin Liu (SW-GPU) <[email protected]>
for more information, see https://pre-commit.ci
Signed-off-by: binliu <[email protected]>
for more information, see https://pre-commit.ci
Signed-off-by: Bin Liu (SW-GPU) <[email protected]>
Signed-off-by: binliu <[email protected]>
Signed-off-by: binliu <[email protected]>
Signed-off-by: binliu <[email protected]>
Signed-off-by: binliu <[email protected]>
Signed-off-by: binliu <[email protected]>
Signed-off-by: binliu <[email protected]>
for more information, see https://pre-commit.ci
Signed-off-by: binliu <[email protected]>
…iunls/MONAILabel into add-vsita2d-to-cellprofiler
for more information, see https://pre-commit.ci
Will look at it soon, thanks, this is great |
sample-apps/vista/README.md
Outdated
limitations under the License. | ||
--> | ||
|
||
# VISTA Application |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's vista2d then better to use sample-apps/vista2d. we have conflicting vista3d also
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do
@@ -467,11 +467,10 @@ def _get_network(self, device, data): | |||
if path: | |||
checkpoint = torch.load(path, map_location=torch.device(device)) | |||
model_state_dict = checkpoint.get(self.model_state_dict, checkpoint) | |||
|
|||
if set(self.network.state_dict().keys()) != set(checkpoint.keys()): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this change break any existing checkpoints for other models? bundles? other apps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. Because of this line model_state_dict = checkpoint.get(self.model_state_dict, checkpoint)
. If the checkpoint is the model_state_dict, then it will return the checkpoint directly. Otherwise it will return the model_state_dict. I think this is why this line exists here. And the old condition logic which uses the checkpoint looks like a bug to me. Please correct me if there are other cases.
Thanks
Bin
Hi @binliunls , the app generally looks good, but why is the vista developed as an app instead of a model? I assume it will be the only model if we consider it as a new app. Can we do the vista2d model as a model in pathology app? |
Hi @tangy5 , actually this should be a monai bundle app since the vista2d is a typical monai bundle from the inference aspect. However, the writter of this vista2d is difference from regular bundles which use I would suggest that we keep this app here until all the vista2d workflows (training, validation, inference and active learning) can be adapted to the MONAI bundle app. Then I can make another PR to delete this app and add some explanation of how to start vista2d with MONAI bundle app. Thanks |
Signed-off-by: binliu <[email protected]>
Good, I think the app design is good. Refactoring to a model and customization will be too much, it isn't worth the effort. |
@@ -0,0 +1,10 @@ | |||
# Copyright (c) MONAI Consortium |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The trainer is empty now right?
And we only support inference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we currently only need inference for this APP. If there is a requirement for trainning, we can also add it.
@binliunls , before merge, please make sure all workflow are working normally about Ceelprofiler. Let's talk about about moving the vista2d mode to monaibundle app later. And please keep consistent on the name "VISTA2d" over "vista" as Sachi mentioned. Thank you |
Signed-off-by: binliu <[email protected]>
Signed-off-by: binliu <[email protected]>
Signed-off-by: binliu <[email protected]>
for more information, see https://pre-commit.ci
Created a PR in the cellprofiler plugin repo for the cellprofiler CI test. |
Hi @binliunls , Could you please help also develop a tutorial and submit to: Thanks in advance. |
Signed-off-by: binliu <[email protected]>
Merged Project-MONAI/tutorials#1803 |
No description provided.