Closed romainVala closed 6 years ago
effigies
commented
6 years ago Thanks for the report. We will indeed need to work around this, but it would be worth notifying Freesurfer, as well. Their issues page is not very active, so you may prefer to email the mailing list.
effigies
commented
6 years ago Would you be interested in submitting a pull request to fix this issue?
I think it could be done with a fairly simple interface that we place after the mri_coreg/bbregister nodes. I'd be glad to help you through the process.
oesteban
commented
6 years ago This is somewhat related to #768, isn't it?
effigies
commented
6 years ago Oh yes. I knew we'd done something similar, but hadn't realized how similar. So we could patch BBRegister and MRICoreg interfaces with the same adjustment.
romainVala
commented
6 years ago If you do not have a quick solution I can give a try ... but I am not familiar with nipype, if you could guide me where to look, or how can I see the changes you made for the similar problem?
effigies
commented
6 years ago The fix was here: https://github.com/poldracklab/fmriprep/pull/778/files
The current version of that fix can be found here:
So a fix for this would look like:
1) Create PatchedBBRegister and PatchedMRICoreg classes, subclassing from fs.BBRegister and fs.MRICoreg, respectively.
2) Find the out_lta (or appropriately named output file) in a _list_outputs(), and filter it for lines with >=255 characters, rewriting the file, if needed.
If you're interested, feel free to open a pull request at any time, and ask any questions you might have.
romainVala
commented
6 years ago Hi thanks I start to have a loock and I am not sure to look at the wrigh files
for fs.BBRegister you refer to the class define in niworkflows-0.4.3-py3.6.egg/niworkflows/interfaces/registration.py class BBRegisterRPT
if yes I will need to make a pull request also in https://github.com/poldracklab/niworkflows/ ?
Sory for the confusion
effigies
commented
6 years ago Ah, yes, I'd somewhat forgotten about that aspect of it. I think for now let's stay in fMRIPrep. We can create a mixin like this:
class TruncateLTA(object):
# Use a tuple in case some object produces multiple transforms
lta_outputs = ('out_lta_file',)
def _post_run_hook(self, runtime):
runtime = super(TruncateLTA, self)._post_run_hook(runtime)
outputs = self.aggregate_outputs(runtime=runtime)
for lta_name in self.lta_outputs:
lta_file = outputs[lta_name]
if not isdefined(lta_file):
continue
...
# Copy the bits that do the work from above, using lta_file
# instead of outputs['out_file']
...
return runtimeThis takes advantage of post-run-hooks in Nipype interfaces, which are guaranteed to run after the interface has completed, and before any unused files are cleaned up.
Now instead of writing the same patch for each interface, we can rewrite PatchedConcatenateLTA like so:
class PatchedConcatenateLTA(TruncateLTA, ConcatenateLTA):
lta_outputs = ['out_file'](You may need to swap the order of TruncateLTA and ConcatenateLTA to make it work properly, but I think this is right.)
And then you can import BBRegisterRPT and MRICoregRPT from niworkflows, and patch them similarly into PatchedBBRegisterRPT and PatchedMRICoregRPT.
Does this all make sense? Please ask as many questions as you need.
romainVala
commented
6 years ago Ok I made just PatchedBBRegisterRPT and PatchedMRICoregRPT I hope it is fine, at least it works localy I did not put too much comment ... todo ... it is my first pull request, not sure also about the process
I'll see on monday ++
effigies
commented
6 years ago Fixed in #1274.
Hello
I hope it is the wright place to report the bug : I get systematic Segmentation fault for mri_vol2vol.bin when performing the corregistration of the bold data onto the freesurfer T1.
I then notice that by just changing the content of registration.lta, the exact same mri_vol2vol command would work :
Here is my registration file :
The problem comes from
if I then change it to
then it just works fine
note that the filename path is no more correct, but just a little shorter. mri_vol2vol result look fine (it is well corregister to freesufer brain_mask and reslice at the same resolution)
So a solution would be to change the absolut path by just the filename I hope it can be done easily
Regards
Romain