Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip QC if label file is empty #61

Merged
merged 4 commits into from
Sep 15, 2023
Merged

Conversation

valosekj
Copy link
Member

Check if the label file is empty (i.e., no lesion was drawn in MS patient); if so, skip QC.

Resolves: #60

@NathanMolinier
Copy link
Contributor

By doing that we accept the creation of empty masks for all the tasks. The user needs to be aware of that maybe.

@valosekj
Copy link
Member Author

By doing that we accept the creation of empty masks for all the tasks. The user needs to be aware of that maybe.

Good point! Do you think it would be better to allow the empty mask only for the FILES_LESION key?

@jcohenadad
Copy link
Member

also, don’t we want to check MS lesion mask even if it is empty? otherwise we will miss false negatives.

@valosekj
Copy link
Member Author

also, don’t we want to check MS lesion mask even if it is empty? otherwise we will miss false negatives.

This is an excellent point! I initially thought that sct_qc does not allow to create QC if the mask is empty. But I was mistaken! The empty mask is not allowed for the sct_qc -p sct_label_utils flag. But, with the sct_qc -p sct_deepseg_lesion flag, empty QC can be created; investigation here.

@valosekj
Copy link
Member Author

The QC is now skipped only for functions which do not support empty labels (sct_label_utils, sct_label_vertebrae, sct_detect_pmj, commit). In other words, an empty label mask is supported for sct_deepseg_sc, sct_deepseg_gm, and sct_deepseg_lesion.

@NathanMolinier
Copy link
Contributor

Another point is, do we want to keep empty masks for every tasks ?

@valosekj
Copy link
Member Author

Another point is, do we want to keep empty masks for every tasks ?

Good point! I would say that it is the responsibility of the user to correct/create a label when using the manual_correction.py script. If the user decides to keep the label empty, for example, in case of no MS lesion or no SC compression, it is absolutely fine. But it is true that, for instance, for disc labeling, the empty mask does not make much sense. However, I would not complicate this PR since this PR is about preventing the QC from failing.

@NathanMolinier
Copy link
Contributor

I understand but this falling is also currently the only warning/error that we have regarding empty masks

@valosekj
Copy link
Member Author

I understand but this falling is also currently the only warning/error that we have regarding empty masks

I added the following warning:

logging.warning(f"File {fname_label} is empty. Skipping QC.\n")

Copy link
Contributor

@NathanMolinier NathanMolinier left a comment

Choose a reason for hiding this comment

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

The solution seems to be good to me, I think can merge this PR 🎉

@valosekj valosekj merged commit 6027fac into main Sep 15, 2023
1 check passed
@valosekj valosekj deleted the jv/60-skip_qc_if_label_is_empty branch September 15, 2023 19:40
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.

Error raised by sct_qc when dealing with empty masks
3 participants