DeepRegNet / DeepReg

Medical image registration using deep learning
Apache License 2.0
568 stars 77 forks source link

Demo review pre-r1 #321

Closed YipengHu closed 4 years ago

YipengHu commented 4 years ago

Issue description

Review the existing demos - due 27th (sorry).


@RemiDelaunay classical_mr_prostate_nonrigid paired_mrus_brain

@ZheMin-1992 grouped_mask_prostate_longitudinal unpaired_ct_abdomen

@agrimwood grouped_mr_heart paired_mrus_prostate

@QianyeYang: classical_ct_headneck_affine unpaired_ct_lung

@NMontanaBrown paired_ct_lung unpaired_mr_brain

@mathpluscode unpaired_us_prostate_cv


Things to check:

Things to suggest:

How to review

Preferably, create a branch and a [WIP] PR, such that the change is clear. If in doubt, leave a comment below.

Type of Issue

Please delete options that are not relevant.

mathpluscode commented 4 years ago

@zacbaum @QianyeYang @RemiDelaunay @agrimwood @NMontanaBrown @ZheMin-1992 Hi all, when reviewing the demo, please adapt the README file to this template https://deepreg.readthedocs.io/en/325-improve-contributing-pages/demo/readme_template.html raw file is here https://raw.githubusercontent.com/DeepRegNet/DeepReg/325-improve-contributing-pages/docs/source/demo/readme_template.md

Especially the order or sections, some texts which should be the same.

The main changes are as follows:

In case some section is missing:

  1. if Pre-trained Model is missing, please write "Pre-trained model is not available." and raise a new issue to request it.

  2. if the tested deepreg version is missing, well as reviewer of demo, you should test it and add the version

  3. if references is missing, please check if we should add one, if not, do not add this section

NMontanaBrown commented 4 years ago

@YipengHu

Preferably, create a branch and a [WIP] PR, such that the change is clear.

A bit confused, branch from main or from...?

YipengHu commented 4 years ago

@YipengHu

Preferably, create a branch and a [WIP] PR, such that the change is clear.

A bit confused, branch from main or from...?

from main or any other one that is working on this

NMontanaBrown commented 4 years ago

Generally, with the new template that says:

Please read the DeepReg Demo Disclaimer

What is this meant to point to? This link is broken and has been added to all the demo READMEs but doesn't point to anything...? (https://github.com/DeepRegNet/DeepReg/blob/master/demos/README.md)

Should this be changed to point to readthedocs? @YipengHu @mathpluscode

zacbaum commented 4 years ago

@RemiDelaunay @ZheMin-1992 @agrimwood @QianyeYang @NMontanaBrown @mathpluscode :

Now that the pypi release is available (pip install deepreg) - I would ask that you use that version of the codebase (either via pip install or by downloading the release) to test the demos and indicate that things worked with that version (currently v0.1.6).

Then, we can periodically do a quick re-run of the demos upon new releases, or as we see fit.

mathpluscode commented 4 years ago

@RemiDelaunay @ZheMin-1992 @agrimwood @QianyeYang @NMontanaBrown @mathpluscode :

Now that the pypi release is available (pip install deepreg) - I would ask that you use that version of the codebase (either via pip install or by downloading the release) to test the demos and indicate that things worked with that version (currently v0.1.6).

Then, we can periodically do a quick re-run of the demos upon new releases, or as we see fit.

This sounds nice, for the demos we definitely need a script to test all demos automatically.

I raised this issue https://github.com/DeepRegNet/DeepReg/issues/344, please feel free to add more ideas ;)