Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added disc labels for T1w and T2w #15

Merged
merged 5 commits into from
Aug 27, 2020

Conversation

lrouhier
Copy link
Contributor

@lrouhier lrouhier commented Aug 26, 2020

This PR aims at adding files containing discs label for most subjects.
All centers have been labeled except:

  • fslPrima
  • fslAchieva
  • mountSinai

It also provides one minor correction to CONTRIBUTION.md.

@jcohenadad
Copy link
Member

@lrouhier could you please attach the QC report of these added derivatives, for easy review and track record (the reports are always attached to new releases). Thanks

@jcohenadad jcohenadad changed the title Lr/add labels Add disc labels Aug 26, 2020
@lrouhier
Copy link
Contributor Author

@jcohenadad
Copy link
Member

Qc folder: https://drive.google.com/drive/folders/1_HB7tpg-G_Xq6tQ_nQ1XatBKk8lXSNRf?usp=sharing

@lrouhier could you instead point to a zipped file (otherwise the google drive API needs to do the zip each time a download is requested-- which is a waste of time)-- thanks

@lrouhier
Copy link
Contributor Author

Here is a zip link to the qc: https://drive.google.com/file/d/1wDNaCKI4SaTJVmzS_v-zez0SIHdJyEzW/view?usp=sharing

@jcohenadad
Copy link
Member

damn! travis gets stuck with a timeout, which we didn't get in the past (i already restarted the job):
https://travis-ci.com/github/spine-generic/data-multi-subject/builds/181479798#L563

@lrouhier
Copy link
Contributor Author

Yes I saw ! Then again that's 484 more files, even if it's not that heavy (because it's just label) I'm guessing it's still adding like a minute or two which might be enough to go overboard.
I'm guessing is due to the fact that the job has no log output (https://docs.travis-ci.com/user/customizing-the-build#build-timeouts), because we use a quiet mode and it takes more than 10 minutes now so it is shutting down.

@jcohenadad
Copy link
Member

@lrouhier i've noticed a few problems:

  • On the T2w images, the labels are often placed too inferiorly with respect to the posterior tip of the disc
  • On the T2w images, some labels are placed inside the central canal (instead of the posterior tip)
  • Some labels are missing where discs are visible

I haven't reviewed all of them (only the first ~10%), but I expect the same problem is present in the remaining 90%.

Here are some screenshots of notable problems: problem_discs.zip

@lrouhier
Copy link
Contributor Author

lrouhier commented Aug 26, 2020

Yes I saw. I am correcting that.

Might be due to the fact that I did not label on the same slice as the displayed slice (which can cause a few pixels of difference)

@jcohenadad
Copy link
Member

Might be due to the fact that I did not label on the same slice as the displayed slice (which can cause a few pixels of difference)

ah! good point. Can you please verify if the displayed slice is the same as the one you used, and if not, let's fix that first

@lrouhier
Copy link
Contributor Author

I checked and it does not seem to be that. I am honestly a bit confused. I Check my "straight" label that usually use and these are correct. SO I may have worked with T2 registered to T1 to try and use the T1 label and then corrected on either the T2 registered to T1 or the straighten image. In any case, all the labels have been corrected I will produce the qc In a minute just after I update this PR (so that the data matches the QC)

@jcohenadad
Copy link
Member

@kousu can we authorize "squash and merge" for the repos, so the addition/deletion of binaries in non-master branches can be "forgotten" from the history tree?

@lrouhier
Copy link
Contributor Author

@lrouhier
Copy link
Contributor Author

FYI: with the correction we added the missing subjects so all should be labeled.

@jcohenadad
Copy link
Member

@PaulBautin could you please review?

@PaulBautin
Copy link
Contributor

@lrouhier, I'm not finished reviewing but i came across this image. I guess this is just due to the QC image slice:
image

@jcohenadad
Copy link
Member

@lrouhier, I'm not finished reviewing but i came across this image. I guess this is just due to the QC image slice:
image

indeed, this is a known problem. This PR needs to be ported from the old repos to the new repos: spine-generic/data-multi-subject_DO-NOT-USE#23

I'll do it when i find a minute

@jcohenadad
Copy link
Member

defacing problem addressed in #19.

@lrouhier
Copy link
Contributor Author

New qc -->https://drive.google.com/file/d/1LHdernT1sW350IMQ0uSc0mcC_KxP0uLw/view?usp=sharing
sub-Tokyo750w01 now with more label (as per request by @PaulBautin) Tell me it is good now!

@PaulBautin
Copy link
Contributor

Ok, everything looks great. Thanks @lrouhier ! Once this PR and PR #19 are merged we will be ready for a new csa-atrophy processing.

Copy link
Member

@jcohenadad jcohenadad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 👍 Thank you @lrouhier for this awesome contribution!! 🏅

@jcohenadad jcohenadad merged commit 6ffbb5c into spine-generic:master Aug 27, 2020
@joshuacwnewton joshuacwnewton modified the milestone: r20200826 Aug 28, 2020
@jcohenadad jcohenadad changed the title Add disc labels Add disc labels for T1w and T2w Sep 4, 2020
@jcohenadad jcohenadad added this to the next-release milestone Sep 15, 2020
@jcohenadad jcohenadad changed the title Add disc labels for T1w and T2w Added disc labels for T1w and T2w Sep 15, 2020
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