MetricsReloaded icon indicating copy to clipboard operation
MetricsReloaded copied to clipboard

Implementation details and fixes

Open brudfors opened this issue 3 years ago • 1 comments

  • To decrease code repetition and chance of implementation errors some functions/classes can be moved to a utility library: https://github.com/csudre/MetricsReloaded/blob/mikael-mods/utils.py and imported when needed, i.e., https://github.com/csudre/MetricsReloaded/blob/f7f6296468b6556c882cccebb8b6a973d2352e01/pairwise_measures.py#L6 This could also helps in highligting dependencies that would have to be dealt with for a potential MONAI conversion (e.g., skeletonize).
  • Parenthesis missing around shape argument, fix: https://github.com/csudre/MetricsReloaded/blob/f7f6296468b6556c882cccebb8b6a973d2352e01/pairwise_measures.py#L428
  • Ensure ref and pred are binarised, fix: https://github.com/csudre/MetricsReloaded/blob/f7f6296468b6556c882cccebb8b6a973d2352e01/pairwise_measures.py#L180 https://github.com/csudre/MetricsReloaded/blob/f7f6296468b6556c882cccebb8b6a973d2352e01/pairwise_measures.py#L181
  • Euclidean distance transform errors as it has two return variables by default, fix: https://github.com/csudre/MetricsReloaded/blob/f7f6296468b6556c882cccebb8b6a973d2352e01/utils.py#L104
  • Centre of mass function name same as variable name, fix (has also been moved to utils.py): https://github.com/csudre/MetricsReloaded/blob/f7f6296468b6556c882cccebb8b6a973d2352e01/utils.py#L93
  • Maybe include a flag for enabling/disabling the debugging print statements?
  • Note that np.percentile interpolates, c.f., Fig 7 in Common... paper that points to a discrete value.
  • Using a histogram in all_multi_threshold_values might be cleaner, i.e. https://github.com/csudre/MetricsReloaded/blob/f7f6296468b6556c882cccebb8b6a973d2352e01/prob_pairwise_measures.py#L80
  • Added a function x_at_y for less repeated code: https://github.com/csudre/MetricsReloaded/blob/f7f6296468b6556c882cccebb8b6a973d2352e01/utils.py#L107 i.e., https://github.com/csudre/MetricsReloaded/blob/f7f6296468b6556c882cccebb8b6a973d2352e01/prob_pairwise_measures.py#L236
  • No need to load image data to get nifti shape, i.e. https://github.com/csudre/MetricsReloaded/blob/f7f6296468b6556c882cccebb8b6a973d2352e01/mixed_measures_processes.py#L142
  • The per_label_dict functions in the classes in mixed_measures_processes.py could perhaps be implemented in a base class, which would lead to less lines of code.

brudfors avatar Oct 09 '22 14:10 brudfors

@brudfors - util function is a good idea - will modify to that purpose Regarding the x_at_y - good point but direction of monotonicity need to be acknowledged Can't use the histogram due to need to create bins to have equal number of samples

csudre avatar Oct 09 '22 15:10 csudre