Skip to content

Conversation

@rijobro
Copy link
Contributor

@rijobro rijobro commented Dec 17, 2020

Unify visualisation classes.

Description

Beforehand, occlusion sensitivity was a function in metrics, whereas gradcam functionality was in classes inside the visualize module. Now, both can be called with similar calls:

from monai.networks.nets import densenet121
from monai.visualise import OcclusionSensitivity
from monai.visualise import CAM
model_2d = densenet121(spatial_dims=2, in_channels=1, out_channels=3)

cam = CAM(nn_module=model_2d, target_layers="class_layers.relu", fc_layers="class_layers.out")
result_cam = cam(x=torch.rand((1, 1, 48, 64)))

occ_sens = OcclusionSensitivity(nn_module=model_2d)
result_occ_sens = occ_sens(x=torch.rand((1, 1, 48, 64)), class_idx=None, b_box=[-1, -1, 2, 40, 1, 62])

We also had mixed American and British spelling, so changed all instances of visualize to visualise. Maybe I should have gone the other way, as it made lots of changes...

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh --codeformat --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro rijobro force-pushed the unify_visualisation branch from 8878366 to 88abbe7 Compare December 17, 2020 17:06
@wyli
Copy link
Contributor

wyli commented Dec 17, 2020

I feel we should go for visualize... to be consistent with pytorch and numpy
thanks for raising this!

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro
Copy link
Contributor Author

rijobro commented Dec 17, 2020

ok, updated.

@rijobro rijobro mentioned this pull request Dec 17, 2020
6 tasks
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@wyli
Copy link
Contributor

wyli commented Dec 18, 2020

thanks for the clean up! but I think it's too early to define a proper NetVisualizer API, we should keep the module simple for now, and it needs more discussions along with #528

@rijobro
Copy link
Contributor Author

rijobro commented Jan 4, 2021

but I think it's too early to define a proper NetVisualizer API

I can remove it if you'd like but it's only a skeleton at the moment, with a constructor and __call__ method. Just so that when future visualisations get added, they are more likely to conform (avoid the difference we saw with OccSens and GradCAM).

Up to you, I'll delete the base class if you'd like, but I'm in favour of keeping it.

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@wyli
Copy link
Contributor

wyli commented Jan 4, 2021

but I think it's too early to define a proper NetVisualizer API

I can remove it if you'd like but it's only a skeleton at the moment, with a constructor and __call__ method. Just so that when future visualisations get added, they are more likely to conform (avoid the difference we saw with OccSens and GradCAM).

Up to you, I'll delete the base class if you'd like, but I'm in favour of keeping it.

I feel the benefit of having a NetVisualizer class is not very clear at the moment (and the interface is also not well documented), all the other changes look very nice. perhaps @Nic-Ma @ericspod could have more comments.

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro
Copy link
Contributor Author

rijobro commented Jan 4, 2021

Ok, I can remove it.

I also created a train_mode context manager (opposite of eval_mode) here. We can use this when calculating the gradient for GradCam, which should avoid the problem you saw here: Project-MONAI/tutorials#83 (comment).

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@wyli wyli merged commit a74fef9 into Project-MONAI:master Jan 6, 2021
@rijobro rijobro deleted the unify_visualisation branch January 6, 2021 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants