ivadomed / utilities

Repository containing utility scripts for handling various aspects in a DL model training pipeline
MIT License
8 stars 0 forks source link

Add python script and quick start guide for MetricsReloaded #46

Closed valosekj closed 6 months ago

valosekj commented 8 months ago

Description

This PR adds:

Useful links:

valosekj commented 8 months ago

When digging into the MetricsReloaded code, @naga-karthik and I did the following:

valosekj commented 8 months ago

Edge cases (using this PR):

  1. Empty reference and empty prediction --> model learned correctly
{
    "reference": "ref.nii.gz",
    "prediction": "pred.nii.gz",
    "0": {
        "dsc": 1,
        "fbeta": 1,
        "nsd": NaN,
        "vol_diff": NaN,
        "rel_vol_diff": 0,
        "EmptyRef": true,
        "EmptyPred": true
    }
}
  1. Empty reference and non-empty prediction --> model did not learn correctly (assuming that the GT is correct) --> model is over-segmenting
{
    "reference": "ref.nii.gz",
    "prediction": "pred.nii.gz",
    "1.0": {
        "dsc": 0.0,
        "fbeta": 0,
        "nsd": 0.0,
        "vol_diff": Infinity,
        "rel_vol_diff": 100,
        "EmptyRef": true,
        "EmptyPred": false
    }
}
  1. Non-empty reference and empty prediction --> model did not learn correctly --> model is under-segmenting
{
    "reference": "ref.nii.gz",
    "prediction": "pred.nii.gz",
    "1.0": {
        "dsc": 0.0,
        "fbeta": 0,
        "nsd": 0.0,
        "vol_diff": 1.0,
        "rel_vol_diff": -100.0,
        "EmptyRef": false,
        "EmptyPred": true
    }
}
naga-karthik commented 8 months ago

Regarding 2:

Empty reference and non-empty prediction

The 100 is a bit weird given the the reference is empty so a division by an empty array should be Infinity, but I understand that we need to show that this is oversegmentation so you set 100 here

valosekj commented 8 months ago

The 100 is a bit weird given the reference is empty so a division by an empty array should be Infinity,

Yes, you are right; without this condition, the RVE would be Infinity. Which would complicate aggregation across subjects, though.

but I understand that we need to show that this is oversegmentation so you set 100 here

Yeah, I choose 100 to be the opposite of -100 (which results from non-empty reference and empty prediction).

valosekj commented 8 months ago

Hey @naga-karthik! I believe the PR can now be beta-tested.

I implemented the following points based on our discussions:

I also updated the handling of the case when both the reference and prediction are empty in https://github.com/ivadomed/utilities/pull/46/commits/a23b81bbf6020397098c5b450b082a1ef7b23896.

naga-karthik commented 7 months ago

So I am trying out the installation of MetricsReloaded and followed the commands mentioned here. However, I get a pip dependency resolver error post-installation

Error ![Screenshot 2024-03-20 at 9 58 34 PM](https://github.com/ivadomed/utilities/assets/53445351/4f3f2f68-5d47-46f5-afd2-339b35248f1a)

Is it something you also had? How did you solve this? @valosekj

valosekj commented 7 months ago

So I am trying out the installation of MetricsReloaded and followed the commands mentioned here. However, I get a pip dependency resolver error post-installation

I just tried the commands and the installation completed successfully on my end. We can check it together in person.

valosekj commented 6 months ago

CI is passing and I self-reviewed the code --> merging