Feature/improve metric calculations #45
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… on each individual slice
jonaskordt
reviewed
Dec 15, 2021
| "train_metrics": ["dice_score"], | ||
| "train_metric_confidence_levels": [0.25, 0.5, 0.75], | ||
| "test_metrics": ["dice_score", "sensitivity", "specificity", "hausdorff95"], | ||
| "test_metric_confidence_levels": [0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9] |
There was a problem hiding this comment.
All these should be documented in the README
| metrics: Iterable[str], | ||
| confidence_levels: Iterable[float], | ||
| image_ids: Iterable[str], | ||
| slices_per_image: int, |
There was a problem hiding this comment.
This will be problematic for the Medical Segmentation Decathlon data. There, images for the same segmentation task can have different amount of slices.
src/datasets/dataset_hooks.py
Outdated
| """ | ||
|
|
||
| @abstractmethod | ||
| def slices_per_image(self, **kwargs): |
There was a problem hiding this comment.
Missing return type annotation.
See my other comment about this being a single int.
| @@ -109,15 +112,20 @@ def run_active_learning_pipeline( | |||
| else: | |||
| raise ValueError("Invalid data_module name.") | |||
|
|
|||
| if checkpoint_dir is not None: | |||
| checkpoint_dir = os.path.join(checkpoint_dir, f"{wandb_logger.experiment.id}") | |||
There was a problem hiding this comment.
We should use a similar naming for the prediction directory
jonaskordt
approved these changes
Dec 17, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #32, #37, #38
May also contribute to #25, #43, #44
This PR implements the following features:
train_metrics,train_metric_confidence_levels,test_metricsandtest_metric_confidence_levelswere added to specify which metrics should be tracked for each training step and which should only be tracked for the best modelmodel_selection_criterionwas introduced to specify which metric should be used to select the best model from each training runWith the changes from this PR, a training epoch on the BraTS 2018 dataset takes approximately three minutes on an A100 GPU and the model evaluation at the end of the training takes approximately five minutes. During each training epoch, the Dice score is computed for three confidence levels (0.25, 0.5, 0.75) on both the training and the validation set. During model evaluation, the Dice score, sensitivity, specificity and the Hausdorff distance are computed for nine confidence levels (0.1 - 0.9) on the validation set using the best model.
To ensure that the metric tracking is working in all cases, I would like to extend the test cases in a follow-up PR.