577 update MeanDice handler

[Nic-Ma]

Fixes #577 .

Description

This PR updated the MeanDice handler based on new DiceMetric class API.
And updated all the examples to use DiceMetric instead.

Status

Ready

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• Breaking change (fix or new feature that would cause existing functionality to change)
• New tests added to cover the changes
• Docstrings/Documentation updated

[Nic-Ma]

Hi @wyli ,

I got several questions about the new added DiceMetric class:

1. Why it changed the behavior for Nan? In the previous practice, we skipped Nan values, but the DiceMetric changed Nan to 0 for computation.
2. When computing average, In the previous practice, we skipped if not_nan.sum()=0, but the DiceMetric use it to div:
   https://github.com/Project-MONAI/MONAI/blob/master/monai/metrics/meandice.py#L81
   So all the tests related to MeanDice are affected.
3. There is no mean_channel in the DiceMetric, how to compute overall meandice except set reduction="None"? I think the count of last batch may be smaller than others, so we can't use "mean" or "mean_batch" to compute the overall meandice.
4. What's the use case of reduction="mean" or reduction="mean_batch" as it only computes average on 1 batch?

Could you please help me understand these questions? Then I can move on to complete this PR.

Thanks.

[wyli]

I think 1, 2 are good 'style' issues, I don't have any preference, we just need to be consistent within the project. Perhaps @myron could have further comments. For 3, reduction="mean_channel" sounds good. For 4 I guess it'll be easier to compare the score directly with the loss, couldn't think of any side effects

[Nic-Ma]

I think 1, 2 are good 'style' issues, I don't have any preference, we just need to be consistent within the project. Perhaps @myron could have further comments. For 3, reduction="mean_channel" sounds good. For 4 I guess it'll be easier to compare the score directly with the loss, couldn't think of any side effects

Thanks @wyli for your explanation, do you think it's necessary to also add class API for AUC?

Hi @myron ,

Could you please help share something about my questions?
Thanks in advance.

[wyli]

I think 1, 2 are good 'style' issues, I don't have any preference, we just need to be consistent within the project. Perhaps @myron could have further comments. For 3, reduction="mean_channel" sounds good. For 4 I guess it'll be easier to compare the score directly with the loss, couldn't think of any side effects

Thanks @wyli for your explanation, do you think it's necessary to also add class API for AUC?

Hi @myron ,

Could you please help share something about my questions?
Thanks in advance.

our current AUC is partly aligned with scikit learn, I don't think refactoring AUC is a high-priority task for us.
Multi-class ROC/AUC is less obvious, before refactoring we need to look into the literature such as
http://proceedings.mlr.press/v97/kleiman19a/kleiman19a.pdf
https://link.springer.com/content/pdf/10.1023/A:1010920819831.pdf

[Nic-Ma]

/black

[Nic-Ma]

Hi @wyli ,

Sounds good a plan.
After offline discussion with @myron , I think we achieved agreement on my questions.
And I already updated the code, could you please help review it again?

Thanks.

[Nic-Ma]

/black

[Nic-Ma]

Hi @wyli ,

Thanks for your offline review, I updated according to your comments.
Could you please help review it again?
Thanks.

[wyli]

thanks, the dicemetric handles the 'nan' items with an additional instance variable. this design may need another round of refactoring, but for now it can replicate the previous results

[Nic-Ma]

thanks, the dicemetric handles the 'nan' items with an additional instance variable. this design may need another round of refactoring, but for now it can replicate the previous results

Sure, let's refactor all metrics when the PR #565 is ready.
Thanks.