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

Add anatomical data for testing #2

Open
po09i opened this issue Aug 4, 2020 · 5 comments
Open

Add anatomical data for testing #2

po09i opened this issue Aug 4, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@po09i
Copy link
Member

po09i commented Aug 4, 2020

Anatomical data is required to start implementing masking functions. The data could be unsorted dicom data so that we can also improve our bids config file.

@po09i po09i added the enhancement New feature or request label Aug 4, 2020
@po09i po09i self-assigned this Aug 4, 2020
@jcohenadad
Copy link
Member

The data could be unsorted dicom data so that we can also improve our bids config file.

you need nifti input for unit-testing your masking function, not dicom

also, i would suggest to separate testing as much as possible, i.e.:

  • to test the dicom_to_nifti function: add under unsorted dicom the necessary files (no more, no less than required-- trying to keep the repos as small as possible)
  • to test masking: add nifti file in the already-existing BIDS folder

@po09i
Copy link
Member Author

po09i commented Aug 4, 2020

I'm thinking of adding the acquisition done in ACDC_108:

  • anat: gre_realtime_zshim -- size (68, 128, 20, 8)

Your suggestion is to add an unsorted dicom version of this acquisition and a nifti bids version to facilitate unit tests?

@jcohenadad
Copy link
Member

jcohenadad commented Aug 4, 2020

my suggestion was to separate the tests data when it makes sense.

example:

  • test 1 minimally requires data A
  • test 2 minimally requires data A or B
  • test 3 minimally requires data C

with that in mind:

  • uploading A B C D E F is useless
  • uploading A B C is suboptimal
  • uploading A C is optimal

it sounds trivial, but it is important to always keep that in mind and try to minimize test data size.

example: you're planning on uploading a dataset with 8 echoes, but in fact you only need two so you could do:

  • testing of fieldmap with another sequence with two echoes (because the 6-echo case is already tested with another already-existing data)
  • testing of masking, which only requires one echo

so, why do you need to upload (68, 128, 20, 8)? you could instead get rid of echoes 3->8 so you save 68x128x20x6xdtype space.

the same applies to the XYZ matrix: do you absolutely need the full matrix? can the tests run with equal sensitivity with e.g. 32x32x5 matrix instead?

i think you get my point

@po09i
Copy link
Member Author

po09i commented Aug 5, 2020

Yes this makes things clearer. I'll wait until I start working on the masking function so that I can pinpoint exactly what is required for testing.

@jcohenadad
Copy link
Member

I'll wait until I start working on the masking function so that I can pinpoint exactly what is required for testing.

and in fact, you can also generate data on the fly, e.g. https://github.com/neuropoly/spinalcordtoolbox/blob/master/spinalcordtoolbox/testing/create_test_data.py#L120

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

No branches or pull requests

2 participants