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

Train model with new datasets #49

Closed
4 tasks done
jcohenadad opened this issue Oct 11, 2022 · 15 comments
Closed
4 tasks done

Train model with new datasets #49

jcohenadad opened this issue Oct 11, 2022 · 15 comments
Assignees
Labels

Comments

@jcohenadad
Copy link
Member

jcohenadad commented Oct 11, 2022

@jcohenadad
Copy link
Member Author

I copied the new MP2RAGE dataset under duke:mri/basel/INsIDER_SCT_Segmentations_COR. @kiristern as we discussed would you mind BIDSifying these data and putting them somewhere temporary (eg: duke:temp/kiri) and then I will review and put it in our git-annex data server. Few things:

  • could you confirm you have read access to the data on duke?
  • there are scripts that Alex used to BIDSify the data (you don't have to start from scratch): here: data.neuro.polymtl.ca:/datasets/basel-mp2rage/scripts
  • Importantly: some of the patients overlap with what is in data.neuro.polymtl.ca:/datasets/basel-mp2rage so we should make sure to not duplicate subjects. I think these are the first 35 patients (starts with "P", and controls are "C") but this needs to be verified.
  • I notice that there is only one segmentation file for these updated data, while there are two segmentation files for the 35 patients we already have. I'll ask who did the segmentations so we can mention this in the JSON metadata file
  • On that note: the current dataset of 35 patients has in fact 3 segmentation files. The 3rd is labeled under my name. @uzaymacar do you remember how it was generated? is this 3rd segmentation the majority voting of the first 2? Or is that a 'curated' segmentation revised by me?

@kiristern
Copy link

@jcohenadad sorry for the delay. I just checked and I don't have access to duke/mri

@jcohenadad
Copy link
Member Author

jcohenadad commented Oct 17, 2022

@jcohenadad sorry for the delay. I just checked and I don't have access to duke/mri

on it :-)

Fixed!

@kiristern
Copy link

@jcohenadad data is uploaded to duke/temp/kiri/bidsify.
i wasn't sure what to assign to Author in the json key, so left it blank for now.

@kiristern
Copy link

wondering about issue 1202? I guess for now since no changes have been established, keeping as is.
(However, I just realized the file names contain caps — I’ll modify that tmrw and re-upload with the corrected file names)

@jcohenadad
Copy link
Member Author

@kiristern Thank you for curating the data. I looked at them and the file name does not correspond to the BIDS convention. For example, here's what you created:

sub-P036_MP2R_INsIDER_SCT_Seg_COR.nii.gz

whereas it should look like this one (from our already existing database):

sub-P001_UNIT1.nii.gz

Also, the JSON file has the extention nii.json instead of json.

Also, I suggest you use an already existing script to BIDSify the data (the same that was used for generating the initial 35 patients data, located under scripts/ in the BIDS dataset). Notably, there is a script called curate_basel_additional_data.py, so you could give it a try, and modify it if needed? And then, pls put it under a script/ folder in the dataset you share under duke so we make sure to include your modified script

@kiristern
Copy link

ahh sorry, i misunderstood. i filtered out the duplicate patients based on the same sub-PXXX but didn't realize that the file ending needed to match as well.. makes sense. thanks for noticing the .nii.json too. I'll update this evening.

oh i used the curate_basel.py not _additional_.

@jcohenadad
Copy link
Member Author

No worries, let me know when I can review it again 😊

@kiristern
Copy link

Okay, fixed. but before uploading to server for review, just want to confirm whether "Author" should be the same as the person indicated in the curate_basel_additional_data.py script, or leave blank?

@jcohenadad
Copy link
Member Author

Okay, fixed. but before uploading to server for review, just want to confirm whether "Author" should be the same as the person indicated in the curate_basel_additional_data.py script, or leave blank?

good question. I've just emailed our collaborators to ask. Once we have the answer, please make sure to not edit the py. file, but instead create a new one that will be specific to this dataset. Eg: curate_basel_additional_data_20221028.py and edit the comment at the top of the python file with the relevant info-- thanks

@kiristern
Copy link

kiristern commented Oct 28, 2022

sounds good, thanks. And yes, I actually made a new script and saved in on a new branch in the model_seg_ms_mp2rage repo

@jcohenadad
Copy link
Member Author

sounds good, thanks. And yes, I actually made a new script and saved in on a new branch in the model_seg_ms_mp2rage repo

could you please open a PR? I already have some comments to give on the script

@jcohenadad
Copy link
Member Author

Follow-up on #49 (comment), both the SC segmentations and MS lesion segmentations were performed by "Tsagkas Charidimos".

@kiristern
Copy link

okay i uploaded the corrected data with Tsagkas Charidimos as the author to duke/temp/kiri/bidsify and i'll create a PR now

@jcohenadad
Copy link
Member Author

Done 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants