Potential bug in the dataset split in the conversion script

[rohanbanerjee]

Hello, I see a potential problem with the dataset split in the conversion script especially through the following line: https://github.com/ivadomed/data-conversion/blob/90e1b5efa0700fb4288e1c182b4eb5836228aa85/dataset_conversion/convert_bids_to_nnunet.py#L133

Explanation

The MSD the dataset only has two sets - Train {imagesTr and lablesTr} and Test {imagesTs and labelsTs} whereas the ivadomed creates three sets - train, validation and test. Now while we define the splits in the ivadomed config file, we define it as the following:

"split_dataset": {
        "fname_split": null,
        "random_seed": 2,
        "split_method" : "participant_id",
        "balance": null,
        "train_fraction": 0.8,
        "test_fraction": 0.1
    }

Here we define the train and test fraction and the rest of it is used for the validation. So from the above example, the train_fraction = 0.80, validation_fraction = 0.1 and the test_fraction = 0.1. In the line that I have mentioned above, appends the train and validation together to make the test set and makes the test as test for the nnUNet. So in the end the train:test fraction becomes 90:10 but should be 80:20 which might cause difference in results.

Potential fix

valid_train_imgs.append(splits["train"])
valid_train_imgs.append(splits["valid"])
valid_test_imgs.append(splits["test"])

In the above snippet, the valid set should be appended to valid_test_imgs instead of valid_train_imgs.

Any inputs on this?

[naga-karthik]

Hey, thanks for opening the issue!

Here we define the train and test fraction and the rest of it is used for the validation.

Are you absolutely sure this is the case? In my experience, the train_fraction is the combination of both train and validation and test_fraction is just used for testing. So, when we define:

train_fraction: 0.8
test_fraction:0.2

It automatically takes the validation subjects from the 0.8 defined in train_fraction.

Once the above this confirmed, we can come to this:

the valid set should be appended to valid_test_imgs instead of valid_train_imgs.

I think this would be confusing and possibly very susceptible to errors because the imagesTs should not be tampered with at point during the training but if we add validation subjects to this list then there's a high chance of some subjects "leaking" b/w val and test sets. It's best if we avoid this.

We can come up with a better solution for sure! I will not be able to look at it this week. Let me know if you have an update and I can take a look next week!

[naga-karthik]

closing this as the issue seems to have been solved! (please reopen if not)