Issue #557: refactoring models

[mathpluscode]

Description

Rewrite registration models from functions to classes, and use registry.
Moreover, affine model is merged into DDF model and got test covered (it was never tested I believe)

Fixes #557

Demos need to be updated

• grouped_mask_prostate_longitudinal, updated
• grouped_mr_heart, updated
• paired_ct_lung, DGX tmux 0, training, ETA 9am Monday
• paired_mr_brain, will not be updated as the ckpt meant to be improved further, see Demo optimise - paired mrus brain #577.
• paired_mrus_prostate, updated, but need further attention Demo optimise paired mr us prostate #621
• unpaired_ct_abdomen, will not be updated as the ckpt meant to be improved further, see Demo optimise - unpaired abdo CT #552.
• unpaired_ct_lung, PT5
• unpaired_mr_brain, will not be updated as the ckpt meant to be improved further, see Demo optimise unpaired mr brain #620.
• unpaired_us_prostate_cv, DGX tmux

Type of change

What types of changes does your code introduce to DeepReg?

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

• Bugfix (non-breaking change which fixes an issue)
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Documentation Update (fix or improvement on the documentation)
• New feature (non-breaking change which adds functionality)
• Other (if none of the other choices apply)

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.

• I have
  installed pre-commit
  using pre-commit install and formatted all changed files. If you are not
  certain, run pre-commit run --all-files.
• My commits' message styles matches
  our requested structure,
  e.g. Issue #<issue number>: detailed message.
• I have updated the
  change log file
  regarding my changes.
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

[codecov]

Codecov Report

Merging #602 (f63dd8f) into main (7ab590b) will increase coverage by 0.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #602      +/-   ##
==========================================
+ Coverage   99.50%   99.67%   +0.17%     
==========================================
  Files          37       33       -4     
  Lines        2209     2175      -34     
==========================================
- Hits         2198     2168      -30     
+ Misses         11        7       -4     

Impacted Files	Coverage Δ
deepreg/model/loss/image.py	100.00% <ø> (ø)
deepreg/model/loss/label.py	100.00% <ø> (ø)
deepreg/dataset/loader/grouped_loader.py	99.12% <100.00%> (+<0.01%)	⬆️
deepreg/model/__init__.py	100.00% <100.00%> (ø)
deepreg/model/backbone/global_net.py	100.00% <100.00%> (ø)
deepreg/model/network.py	100.00% <100.00%> (ø)
deepreg/predict.py	98.93% <100.00%> (+1.61%)	⬆️
deepreg/registry.py	100.00% <100.00%> (ø)
deepreg/train.py	100.00% <100.00%> (ø)
deepreg/vis.py	100.00% <100.00%> (ø)
... and 1 more

---

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 7ab590b...f63dd8f. Read the comment docs.

[mathpluscode]

ooops, as I changed the network definitions, we may not be able to load pretrained weights :(
Then it is important to have some automated scripts to retrain the models one by one.
As a result, we might not want to merge this PR, as all demos' pretrained weight gonna be outdated.

@YipengHu do you have an estimation on the time required for retraining all models?

[mathpluscode]

@YipengHu @s-sd @zacbaum This PR is ready for review.

Except that I'm retraining all models to regenerate the checkpoints, as change of network architectures makes all ckpts not suitable. Therefore it would be nice/saving time if you review this starting now ^^.

EDIT: actually, there have been many fixed in other PRs, this branch has to include those unmerged fixes for training models. We can come back to this later.

[s-sd]

@mathpluscode looks good to me. If you need help updating any of the demos before merging, let me know. Also, not sure if required but if you need re-approval after updating some more demos, let me know as well.

Oh would I need to update some figures? @s-sd

Yeah, I think would be a good idea to update them. I have a script written which creates and saves all figures so if you want I'd be happy to update figures once you're done updating the demo checkpoints, just let me know once done.

@s-sd is your script uploaded in the repo?

@mathpluscode no, I just have it locally. Should I add it to the repo?

[mathpluscode]

@mathpluscode looks good to me. If you need help updating any of the demos before merging, let me know. Also, not sure if required but if you need re-approval after updating some more demos, let me know as well.

Oh would I need to update some figures? @s-sd

Yeah, I think would be a good idea to update them. I have a script written which creates and saves all figures so if you want I'd be happy to update figures once you're done updating the demo checkpoints, just let me know once done.

@s-sd is your script uploaded in the repo?

@mathpluscode no, I just have it locally. Should I add it to the repo?

Maybe? we can leave that for a separate issue. Would you mind to send me the script via mail/teams.
I may need several modif as not all demos have been updated in the PR. (Because some demos are meant to be improved anyway afterwards and they do now show meaningful results for now.)

[mathpluscode]

I guess I just realised that this branch has no conflict with #559.

So it is ready to be merged before/after #559. As I didn't influence the model part.
@YipengHu @NMontanaBrown could you have a look ^^

[mathpluscode]

@YipengHu are you happy with my replies? @NMontanaBrown do you have any further comments :)

[YipengHu]

@YipengHu are you happy with my replies? @NMontanaBrown do you have any further comments :)

all looks good to me.

[NMontanaBrown]

Let me check, one minute.

[NMontanaBrown]

Can we merge the config docs first? I am still waiting on a few comments/resolutions there. @YipengHu @mathpluscode

[mathpluscode]

Can we merge the config docs first? I am still waiting on a few comments/resolutions there. @YipengHu @mathpluscode

There is no conflicts as this PR does not change config structure ^^. But yes, if you prefer this way. I can wait.

[NMontanaBrown]

Can we merge the config docs first? I am still waiting on a few comments/resolutions there. @YipengHu @mathpluscode

There is no conflicts as this PR does not change config structure ^^. But yes, if you prefer this way. I can wait.

Actually, yes, because affine parameter is no longer passed right? @mathpluscode

[mathpluscode]

Can we merge the config docs first? I am still waiting on a few comments/resolutions there. @YipengHu @mathpluscode

There is no conflicts as this PR does not change config structure ^^. But yes, if you prefer this way. I can wait.

Actually, yes, because affine parameter is no longer passed right? @mathpluscode

oooops, good point.

Affine model is integrated into DDF, so that DDF+GlobalNet = Affine, you are totally right!

[scrutinizer-notifier]

The inspection completed: 98 new issues, 40 updated code elements

[mathpluscode]

Can we merge the config docs first? I am still waiting on a few comments/resolutions there. @YipengHu @mathpluscode

There is no conflicts as this PR does not change config structure ^^. But yes, if you prefer this way. I can wait.

Actually, yes, because affine parameter is no longer passed right? @mathpluscode

I've updated the config docs https://deepreg--602.org.readthedocs.build/en/602/docs/configuration.html, have a look? @NMontanaBrown

[mathpluscode]

I have to merge this now as user has experienced a multi-gpu bug, which got solved here.