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

write sfrom and qform #36

Closed
romainVala opened this issue Jan 9, 2020 · 9 comments
Closed

write sfrom and qform #36

romainVala opened this issue Jan 9, 2020 · 9 comments

Comments

@romainVala
Copy link
Contributor

romainVala commented Jan 9, 2020

I notice that you set (in _write_nifti in io.py)

nii.header['sform_code'] = 0

It is more robust (for other software) to set both sform and qform. I would prefer to have

nii.header['sform_code'] = 1

this will save both sfrom and qform (to the same value)

@fepegar
Copy link
Owner

fepegar commented Jan 9, 2020

It is more robust (for other software) to set both sform and qform

For which software? ITK saves images like this by default and I've had trouble before due to qform and sform being different, and for example NiftyReg and Slicer using one or the other as default.

@romainVala
Copy link
Contributor Author

romainVala commented Jan 10, 2020

Well I do not remember, but I notice some problem in the past with data having only one of the sform or the qform set

To have 2 transform in the nifty header is probably the worst idea for this format, and it confuse a lot and it is source of hidden bugs. So the best way to avoid confusion is to always set the both to the identical affine.

My point is that I do not see any argument for not setting the sform ... it will just be more robust and it won't disturb other software to have it set

Actually spm is the only software I know that use those 2 transform differently : when you do a coregister, spm is modifying the sform (only) and keep the qform unchanged
Note that for such volume when you read them with nibabel, the affine read is the sform (so it is consistent with spm) (although nii.get_sform and nii.get_qform is returning 2 differents affines as it is written in the header)

For such volume as input I would say we should generate identical output header (ie with 2 different affine)

For now since we only store internally the affine (comming from the sform) we can not do it, It is not a big deal in my point of view, since at the end we only want to consider the correct affine. (in the case of volume modified by spm coregister, the correct affine to consider is the sform, and this is the default of nibabel, so every thing is fine) we will just ovewrite the qform, (which I still do not understand how it should be used ...)

So I insist, let write the sform too ... no ?

@fepegar
Copy link
Owner

fepegar commented Jan 10, 2020

I notice some problem in the past with data having only one of the sform or the qform set

I've had trouble when only the sform is set. Using only the qform and setting the sform_code to 0 ensures that there's a unique, valid, rigid transformation in the header.

when you read them with nibabel, the affine read is the sform

This is not necessarily true: https://nipy.org/nibabel/nifti_images.html#choosing-the-image-affine
It depends on the sform_code and qform_code. The system I implemented here is the one used by default in ITK.

Anyway, the write functions in io are not a central part of this project, so I don't think it has a big impact.

@romainVala
Copy link
Contributor Author

As you want, your solution is correct, and coherent within torchio.

(I'll just change the corresponding line, in my fork, it is not a big deal

@fepegar fepegar closed this as completed Jan 13, 2020
@fepegar
Copy link
Owner

fepegar commented Jan 13, 2020

Out of curiosity: are you using the ImagesDataset.save_sample method? I thought it wouldn't be used often. I mostly use it for developing purposes.

@romainVala
Copy link
Contributor Author

Well to be honest, for now I just play around with transform, and I still did not start using it for training a network (it will come soon)
I am afraid, some transformation will take too much times, (motion elastic deformation can takes several second, so compositions will become around 10 seconds.

I am still not sure yet, but one solution would be to perform all augmentation once and write them to the disk ...
(doing this way I could use a cluster environment to perform it in parallel on the different inputs data)

What about you, is the 10 second for each volume not too long for training ?

@romainVala
Copy link
Contributor Author

but an other useful use will be when you want to set the transform parameters, you need to perform them and look at saved transform

So I find this is a very important and useful function

@fepegar
Copy link
Owner

fepegar commented Jan 13, 2020

I'm training on an NVIDIA DGX with 40 cores, so I'm not too worried about that because everything is very parallel. Also, I'm not using the queue at the moment so the data loaders are nicely filled in the background continuously. I set proportion_to_augment to 0.1 for RandomMotion and RandomElasticDeformation, and 0.25 for RandomBiasField. Loading uncompressed files is also faster, if you can afford the storage.

@fepegar
Copy link
Owner

fepegar commented Jan 13, 2020

but an other useful use will be when you want to set the transform parameters, you need to perform them and look at saved transform

Good point. That's why I added the CLI tool:

$ torchio-transform input.nii.gz RandomMotion output.nii.gz

It's still a work in progress, I need to adapt it so that it accepts the kwargs for the transform.
5a4331a

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

No branches or pull requests

2 participants