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

adding testing data for distriploy release #1

Closed
wants to merge 0 commits into from

Conversation

lrouhier
Copy link
Contributor

@lrouhier lrouhier commented Jul 22, 2020

This PR aims at uploading the current testing data used in the ivadomed test to github in order to perform a distriploy release.

Needed images and files:

  • model_unet.pt --> pt file for inference testing (does not really need to be well trained we just test if the code runs)
  • sub-XXX --> Bids folder with multiple contrast (T2w,T2star and T1w)
  • derivatives/sub-XXX --> derivatives folder with seg-manual labels for all contrasts, labels-disc-manual labels for T2w, lesion-manual label for all contrasts (dummy seg),
  • bounding_box.json --> dictionnary to test certain function from the bounding_box.py file
  • results.csv --> results from automate training to test results analysis.
  • dataset_description.json --> needed file to descript dataset
  • participants.csv --> table with subject names and potential metadata
  • 3 config file for train/test/eval command --> We need to separate the 3 (it's json files so it does not take much space)

@@ -0,0 +1 @@
{}
Copy link
Member

Choose a reason for hiding this comment

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

where is this data coming from? is it a public dataset? if not, why not simply using a subject from spine-generic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just the copy of the testing data that was already in ivadomed. I do need to add the new data that will be used in the new tests. There will still be a single subject. I don't think it is a public dataset (this PR ).

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it is a public dataset

can we replace it with another subject then

Copy link
Member

Choose a reason for hiding this comment

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

also: if you don't rebase the unused binaries, please make sure to "squash and merge" for merging this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm looking into it with the test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe. I am trying with the available csf_seg given in the ivadomed/data_example_spinegeneric repo

Copy link
Member

@jcohenadad jcohenadad Jul 22, 2020

Choose a reason for hiding this comment

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

ok, i'm getting confused now, i thought you were looking for a file with suffix _lesion-manual?

i would suggest you list, in the PR's body, all the files that are needed by your tests. Then, we can all think and find the best solution for the listed files (in terms of size, availability, need for manual intervention, etc.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry I will list them.

I was using the csf segmentation as a dummy seg. (I renamed it T2w_lesion_manual).

Copy link
Member

Choose a reason for hiding this comment

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

I renamed it T2w_lesion_manual

that’s exactly the kind of stuff i’d like to avoid. it might bring confusion to a future student

Copy link
Member

Choose a reason for hiding this comment

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

@lrouhier: For the previous testing data: what I did was to create a "fake lesion", ie a blob, in the spinal cord (the subject has healthy).

participants.tsv Outdated
@@ -0,0 +1,2 @@
participant_id sex age first_name last_name date_of_birth date_of_scan pathology data_id institution_id
sub-test001 unknown - unknown unknown - unknwon unknwon unknown unknown
Copy link
Member

Choose a reason for hiding this comment

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

would be better to use a data from which we know the provenance (e.g. spine-generic)

@lrouhier lrouhier closed this Jul 24, 2020
@lrouhier lrouhier force-pushed the lr/distriploy_release branch from ef4bc83 to 8b4fa52 Compare July 24, 2020 17:38
@lrouhier
Copy link
Contributor Author

lrouhier commented Jul 24, 2020

Pull request autamoatically Closed by removing the previous commit to replace subject with one from spine-generic. This was done to avoid having a big repository for a single subject. PR #2 will provide similar changes. In summary, nothing was done.

@jcohenadad
Copy link
Member

jcohenadad commented Jul 24, 2020

before closing this PR, could you please document what was done? I don't see any change in this PR, nor the info about the new dataset

^ scrap that-- info about closing: #1 (comment)

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.

3 participants