Skip to content

571 update paired mr-us prostate demo#582

Merged
YipengHu merged 7 commits into
mainfrom
571-demo-weakly
Dec 28, 2020
Merged

571 update paired mr-us prostate demo#582
YipengHu merged 7 commits into
mainfrom
571-demo-weakly

Conversation

@YipengHu
Copy link
Copy Markdown
Member

@YipengHu YipengHu commented Dec 27, 2020

Description

Update the pretrained model for Paired prostate MR-ultrasound registration demo, together with the relevant config files and train/predict scripts.

Fixes #<issue_number>

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)

Comment thread demos/paired_mrus_prostate/demo_data.py
@mathpluscode
Copy link
Copy Markdown
Member

Do you have any stats result showing the updated ckpt is better? predict script should give some metrics. @YipengHu

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 27, 2020

Codecov Report

Merging #582 (e46442b) into main (1b28d49) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #582   +/-   ##
=======================================
  Coverage   99.23%   99.23%           
=======================================
  Files          33       33           
  Lines        1968     1968           
=======================================
  Hits         1953     1953           
  Misses         15       15           

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 1b28d49...e46442b. Read the comment docs.

@YipengHu
Copy link
Copy Markdown
Member Author

This is largely an update of pre-trained model. @s-sd you should be able to find a good example of visuals from this. #474 #565

@mathpluscode
Copy link
Copy Markdown
Member

another question is about how good do we want, for these models, as i guess the image size is still too small

@YipengHu
Copy link
Copy Markdown
Member Author

another question is about how good do we want, for these models, as i guess the image size is still too small

Good question - i'm digging out the old stats. This one is easier, we do have a good stats. For others, the improvement will be solely qualitative - but i think it would be obvious.

@mathpluscode
Copy link
Copy Markdown
Member

another question is about how good do we want, for these models, as i guess the image size is still too small

Good question - i'm digging out the old stats. This one is easier, we do have a good stats. For others, the improvement will be solely qualitative - but i think it would be obvious.

yup, when dealing with the loading bug, we were comparing the stats and we considered 0.2 -> 0.6 in terms of dice means some improvement ^^

@YipengHu
Copy link
Copy Markdown
Member Author

another question is about how good do we want, for these models, as i guess the image size is still too small

Good question - i'm digging out the old stats. This one is easier, we do have a good stats. For others, the improvement will be solely qualitative - but i think it would be obvious.

yup, when dealing with the loading bug, we were comparing the stats and we considered 0.2 -> 0.6 in terms of dice means some improvement ^^

the numerical improvement was small for this case: dice on gland 067 -> 0.69 and probably not significant. I'd be reluctant to put more effort in this case though, considering the dummy data set.

@mathpluscode
Copy link
Copy Markdown
Member

another question is about how good do we want, for these models, as i guess the image size is still too small

Good question - i'm digging out the old stats. This one is easier, we do have a good stats. For others, the improvement will be solely qualitative - but i think it would be obvious.

yup, when dealing with the loading bug, we were comparing the stats and we considered 0.2 -> 0.6 in terms of dice means some improvement ^^

the numerical improvement was small for this case: dice on gland 067 -> 0.69 and probably not significant. I'd be reluctant to put more effort in this case though, considering the dummy data set.

which demo you mean, the prostate one? bcz you changed 3 files for paired prostate, plus 1 file for masked prostate.

@mathpluscode mathpluscode changed the title 571 demo weakly 571 update paired mr-us prostate demo Dec 28, 2020
@YipengHu YipengHu merged commit 076bf56 into main Dec 28, 2020
@YipengHu YipengHu deleted the 571-demo-weakly branch December 28, 2020 10:30
@NMontanaBrown
Copy link
Copy Markdown
Member

Hi @YipengHu @mathpluscode @s-sd, just catching up on new changes - appreciate the work that's going in to improve demos. However, changelog has not been updated, nor was the PR template used correctly - could we ensure as reviewers/PR submitters that we use it appropriately?

@mathpluscode
Copy link
Copy Markdown
Member

Hi @YipengHu @mathpluscode @s-sd, just catching up on new changes - appreciate the work that's going in to improve demos. However, changelog has not been updated, nor was the PR template used correctly - could we ensure as reviewers/PR submitters that we use it appropriately?

nice point.... maybe we can find a way to have the check automatically.

@YipengHu
Copy link
Copy Markdown
Member Author

Hi @YipengHu @mathpluscode @s-sd, just catching up on new changes - appreciate the work that's going in to improve demos. However, changelog has not been updated, nor was the PR template used correctly - could we ensure as reviewers/PR submitters that we use it appropriately?

nice point.... maybe we can find a way to have the check automatically.

good point indeed - will add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants