DeepRegNet / DeepReg

Medical image registration using deep learning
Apache License 2.0
564 stars 76 forks source link

734 large dice #736

Closed mathpluscode closed 3 years ago

mathpluscode commented 3 years ago

Description

The main issue that dice score > 1 because the added metrics were not normalized by the global batch size.

In this PR, we also double-checked/refactored the implementation of several loss functions and added extensive tests to verify.

Fixes #734

Type of change

What types of changes does your code introduce to DeepReg?

Please check the boxes that apply after submitting the pull request.

Checklist

Please check the boxes that apply after submitting the pull request.

If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

mathpluscode commented 3 years ago

@YipengHu feel free to try again?

codecov[bot] commented 3 years ago

Codecov Report

Merging #736 (2c573f9) into main (bbe77ba) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #736   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           39        39           
  Lines         2455      2464    +9     
=========================================
+ Hits          2455      2464    +9     
Impacted Files Coverage Δ
deepreg/dataset/loader/interface.py 100.00% <ø> (ø)
deepreg/dataset/preprocess.py 100.00% <ø> (ø)
deepreg/loss/deform.py 100.00% <100.00%> (ø)
deepreg/loss/image.py 100.00% <100.00%> (ø)
deepreg/loss/label.py 100.00% <100.00%> (ø)
deepreg/model/network.py 100.00% <100.00%> (ø)
deepreg/predict.py 100.00% <100.00%> (ø)
deepreg/train.py 100.00% <100.00%> (ø)
deepreg/util.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update bbe77ba...2c573f9. Read the comment docs.

mathpluscode commented 3 years ago

@YipengHu I checked the implementation of loss and metric, i.e.

They are different classes calling the same functions.

Then regarding the value under multiple GPUs, the doc is here https://www.tensorflow.org/tutorials/distribute/custom_training#define_the_loss_function

So, in short, there are a lot of work to be done to make it right lol

mathpluscode commented 3 years ago

For now, I removed binary dice from metrics, as we were using loss class as metric.

Will transform existing loss functions and add metric back.