Add loss_fn to IgniteMetric and rename to IgniteMetricHandler

[matt3o]

Description

As explained in #6693 I would like to use the DiceCELoss as a train metric as well. This branch adds a very crude but working version of that.
The added tests, which I copied from the MeanDice metric, do still fail. It would be cool if someone more experienced could check, what needs to be done there.
I ran the code with my full DeepEdit setup and it appears to be working just fine there.

No formatting checks done so far - I want to find out first if this code is useful for others. Docs not updated yet either.

Types of changes

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

[matt3o]

Okay, I will implement the changes this weekend. Just to make sure I am understanding you @wyli and @ericspod correctly:

1. I will delete all the code added so far (so no new class IgniteLossMetric) and
2. move it to IgniteMetric where it will become the new loss_fn parameter
3. I'll add a few new tests for that. If anyone has any ideas what would be could possibly even with the expected outcome, that would be nice.
4. (optional) I'll check out if I can change the name of IgniteMetric to IgniteMetricHandler easily

Sounds good?

[wyli]

Okay, I will implement the changes this weekend. Just to make sure I am understanding you @wyli and @ericspod correctly:

1. I will delete all the code added so far (so no new class IgniteLossMetric) and
2. move it to IgniteMetric where it will become the new loss_fn parameter
3. I'll add a few new tests for that. If anyone has any ideas what would be could possibly even with the expected outcome, that would be nice.
4. (optional) I'll check out if I can change the name of IgniteMetric to IgniteMetricHandler easily

Sounds good?

thanks @matt3o, the plan looks good to me.

[ericspod]

Sounds good?

Sounds good to me too. If changing the name isn't a big problem we should do it but keep IgniteMetric as an alias. We can discuss how to do a deprecation warning for it later as well.

[matt3o]

I started the implementation as discussed. Also I added a first test and copied it over from MeanDice to TestHandlerIgniteMetricHandler. I am using the DiceLoss as the training loss.
However the MeanDice appears to return the inverse value of the loss, so 1 - DiceLoss, i.e. 0.75 (MeanDice) instead of 0.25 (DiceLoss). This means the copied tests fail.
Now my question would be how to handle that. I can of course of simply change the tests to check for the loss result and thus 0.25.
Imho however LossMetric should convert the loss to a metric and thus return 0.75 instead of 0.25. However this might break quite a lot of code.
Shall I just leave this is it is and fix only the expected test values?

[wyli]

Sure, please simply change the expected results, there would be some discrepancies between MeanDice and DiceLoss…

[matt3o]

Finally I need instructions how to generate the alias and how to set the rename warning for IgniteMetricHandler. Apart from that the code is already running and all tests should pass. I updated the class name in the docs, tell me if there is anything more to do.

[wyli]

Finally I need instructions how to generate the alias and how to set the rename warning for IgniteMetricHandler. Apart from that the code is already running and all tests should pass. I updated the class name in the docs, tell me if there is anything more to do.

looks good to me, please add the unit tests to this list https://github.com/Project-MONAI/MONAI/blob/dev/tests/min_tests.py#L75 so that it's skipped when running in a minimal environment

[wyli]

/build

[wyli]

thanks, it looks good to me, premerge tests all good

[wyli]

/build