Back to the main page.
Bug 2096 - implement cifti import/export function
| Status | CLOSED FIXED |
| Reported | 2013-04-08 07:26:00 +0200 |
| Modified | 2015-07-15 13:31:03 +0200 |
| Product: | FieldTrip |
| Component: | fileio |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Mac OS |
| Importance: | P3 normal |
| Assigned to: | Robert Oostenveld |
| URL: | https://github.com/Washington-University/megconnectome/issues/26 |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: | http://bugzilla.fcdonders.nl/show_bug.cgi?id=2865http://bugzilla.fcdonders.nl/show_bug.cgi?id=2734 |
Robert Oostenveld - 2013-04-08 07:26:52 +0200
The ft_sourcewrite function needs to be able to write HCP data to the cifti format. Laura has shared a dense connectome in a mat file, which can be used for testing.
Robert Oostenveld - 2013-04-08 07:29:44 +0200
I have made the test file available on /home/common/matlab/fieldtrip/data/test/bug2096
Robert Oostenveld - 2013-04-08 07:36:51 +0200
I have made a test_bug2096 script mbp> git push fieldtrip master Counting objects: 6, done. Delta compression using up to 2 threads. Compressing objects: 100% (4/4), done. Writing objects: 100% (4/4), 575 bytes, done. Total 4 (delta 2), reused 0 (delta 0) To git@github.com:fieldtrip/fieldtrip.git fdfca49..4985635 master -> master
Robert Oostenveld - 2013-04-08 07:56:57 +0200
TODO: download example CIFTI files from http://www.nitrc.org/plugins/mwiki/index.php/cifti:ConnectivityMatrixFileFormats Dense Connectivity File (9GB) Dense Time Series File (1GB) Parcellated Time Series File (40K)
Robert Oostenveld - 2013-04-18 16:41:13 +0200
Some possibilities for the low-level code are >> which xmlread.m /Applications/MATLAB_R2012b.app/toolbox/matlab/iofun/xmlread.m >> which xmlwrite.m /Applications/MATLAB_R2012b.app/toolbox/matlab/iofun/xmlwrite.m or to use http://www.mathworks.nl/matlabcentral/fileexchange/authors/105885 which wraps around these.
Robert Oostenveld - 2013-04-18 16:44:20 +0200
(In reply to comment #3) I downloaded the demo files, we now have mac001> pwd /home/common/matlab/fieldtrip/data/test/bug2096 mac001> ll total 1268560 -rw-r--r-- 1 roboos 655 547487448 Apr 8 07:29 CP10168_4DEXP_3-Restin_BNN_V1_MEG_icaimagcoh_freq3.mat -rw-r--r-- 1 roboos 655 0 Apr 18 16:43 DenseConnectome.dconn.nii -rw-r--r-- 1 roboos 655 101962880 Apr 21 2011 DenseTimeSeries.dtseries.nii -rw-r--r-- 1 roboos 655 46400 Apr 21 2011 ParcellatedTimeSeries.ptseries.nii note that the DenseConnectome.dconn.nii is not 0 bytes, but 9GB instead. It is still downloading at the moment.
Robert Oostenveld - 2013-06-20 15:49:49 +0200
relevant documentation can be found for nifti-2 on http://nifti.nimh.nih.gov/nifti-1/documentation/ and specifically http://nifti.nimh.nih.gov/nifti-1/documentation/nifti1fields/ and for nifti-2 on http://www.nitrc.org/forum/forum.php?thread_id=2148&forum_id=1941
Robert Oostenveld - 2013-06-20 15:50:22 +0200
I just pushed the most recent updates to my public development branch. mbp> git push oostenveld bug2096 Counting objects: 75, done. Delta compression using up to 2 threads. Compressing objects: 100% (59/59), done. Writing objects: 100% (59/59), 10.08 KiB, done. Total 59 (delta 45), reused 0 (delta 0) To git@github.com:oostenveld/fieldtrip.git 9d2013d..fdd38e4 bug2096 -> bug2096
Robert Oostenveld - 2014-09-10 16:00:45 +0200
On 10 Sep 2014, at 01:23, Timothy Coalson wrote in response to an email from me: By the way, in relation to your PS, one thing to make sure to test (since you aim to support cifti-1) is non-square cifti-1 and cifti-2 files that have greater than 1 length in both dimensions. Cifti-1 puts the dimensions into the NIFTI header backwards, requiring a little hackery to read them (in our case, our nifti reader allows overriding the header dimensions on an already-opened file). One way to get such files is to convert the cifti-2 example dtseries to cifti-1 with wb_command. Then you can read them both and compare the matrices. Also, we have never written cifti-1 files with 3 dimensions (and I am probably the only one who has written a 3D cifti in our lab anyway), and the convention I decided on is that the third dimension in cifti-1 goes in the correct place, so only the first two cifti dimensions are swapped in the header in cifti-1. Of course, you may never need to write cifti-1 anyway. ---- Right now the cifti-1 files are only for simple tests. If I plan to support it (reading only, never writing), I'll have to do the conversion as Tim suggests.
Robert Oostenveld - 2014-09-15 10:07:17 +0200
I have been working for a considerable time on a branch in my own github repo, see https://github.com/oostenveld/fieldtrip/tree/bug2096-cifti The structure and API of the code has settled. Most file formats can now be imported, i.e. at least the ones for which I have test data from elsewhere. Last week I had the export of dscalar, dtseries and dconn reviewed by Tim Coalson, who considered them to be ok. Hence I think it is time to merge it in the main fieldtrip trunk (revision 9797) and continue development there. 1) ensured that https://github.com/oostenveld/fieldtrip/tree/master is consistent with svn/HEAD 2) ensured that https://github.com/oostenveld/fieldtrip/tree/bug2096-cifti is consistent with master, i.e. it only contains cifti related changes 3) "git diff master > patch" gives a patchfile, i.e. a file with all diffs compared to master (and thereby to svn/HEAD) Then in fieldtrip-svn, I did mac011> patch -p1 < ../fieldtrip-github-oostenveld/patch patching file fileio/ft_filetype.m patching file fileio/ft_read_cifti.m patching file fileio/ft_read_headshape.m patching file fileio/ft_write_cifti.m patching file fileio/ft_write_headshape.m Hunk #3 succeeded at 55 with fuzz 2. patching file fileio/private/pos2dim.m patching file fileio/private/pos2transform.m patching file fileio/private/read_nifti2_hdr.m patching file fileio/private/write_nifti2_hdr.m patching file ft_connectivityplot.m Hunk #1 succeeded at 38 with fuzz 2. patching file ft_sourcewrite.m Hunk #1 FAILED at 1. 1 out of 2 hunks FAILED -- saving rejects to file ft_sourcewrite.m.rej patching file private/pos2dim.m patching file private/pos2dim3d.m patching file private/pos2transform.m patching file test/test_bug2096.m patching file utilities/ft_datatype.m patching file utilities/ft_datatype_volume.m patching file utilities/private/pos2dim.m patching file utilities/private/pos2transform.m There is an issue with ft_sourcewrite, but that only seems to be in the documentation. mac011> svn status ? @mmatrix M external/mne/mne_ex_read_write_raw.m M fileio/ft_filetype.m ? fileio/ft_read_cifti.m M fileio/ft_read_headshape.m ? fileio/ft_write_cifti.m M fileio/ft_write_headshape.m ? fileio/ft_write_headshape.m.orig ? fileio/private/pos2dim.m ? fileio/private/pos2transform.m ? fileio/private/read_nifti2_hdr.m ? fileio/private/write_nifti2_hdr.m M ft_connectivityplot.m ? ft_connectivityplot.m.orig M ft_sourcewrite.m ? ft_sourcewrite.m.orig ? patch M private/pos2dim.m ? private/pos2dim3d.m M private/pos2transform.m M test/test_bug2096.m M test/test_tutorial_fem.m M utilities/ft_datatype.m M utilities/ft_datatype_volume.m M utilities/private/pos2dim.m M utilities/private/pos2transform.m Bummer, I should have started with a clean fieldtrip-svn copy. Now there are also some unrelated and uncommitted changes, such as in test/test_tutorial_fem.m and external/mne/mne_ex_read_write_raw.m. I had expected the following changes: mac011> grep ^diff ../fieldtrip-github-oostenveld/patch diff --git a/fileio/ft_filetype.m b/fileio/ft_filetype.m diff --git a/fileio/ft_read_cifti.m b/fileio/ft_read_cifti.m diff --git a/fileio/ft_read_headshape.m b/fileio/ft_read_headshape.m diff --git a/fileio/ft_write_cifti.m b/fileio/ft_write_cifti.m diff --git a/fileio/ft_write_headshape.m b/fileio/ft_write_headshape.m diff --git a/fileio/private/pos2dim.m b/fileio/private/pos2dim.m diff --git a/fileio/private/pos2transform.m b/fileio/private/pos2transform.m diff --git a/fileio/private/read_nifti2_hdr.m b/fileio/private/read_nifti2_hdr.m diff --git a/fileio/private/write_nifti2_hdr.m b/fileio/private/write_nifti2_hdr.m diff --git a/ft_connectivityplot.m b/ft_connectivityplot.m diff --git a/ft_sourcewrite.m b/ft_sourcewrite.m diff --git a/private/pos2dim.m b/private/pos2dim.m diff --git a/private/pos2dim3d.m b/private/pos2dim3d.m diff --git a/private/pos2transform.m b/private/pos2transform.m diff --git a/test/test_bug2096.m b/test/test_bug2096.m diff --git a/utilities/ft_datatype.m b/utilities/ft_datatype.m diff --git a/utilities/ft_datatype_volume.m b/utilities/ft_datatype_volume.m diff --git a/utilities/private/pos2dim.m b/utilities/private/pos2dim.m diff --git a/utilities/private/pos2transform.m b/utilities/private/pos2transform.m I stored all of these affected files in a text file out and did mac011> svn add `cat filelist.txt` Some files have been added that should be auto synched: mac011> svn propset autosync true ./fileio/private/pos2*.m property 'autosync' set on 'fileio/private/pos2dim.m' property 'autosync' set on 'fileio/private/pos2dim3d.m' property 'autosync' set on 'fileio/private/pos2transform.m' mac011> svn propset autosync true `find . -name pos2dim3d.m` property 'autosync' set on 'fileio/private/pos2dim3d.m' property 'autosync' set on 'private/pos2dim3d.m' mac011> svn propset autosync true `find . -name pos2transform.m` property 'autosync' set on 'fileio/private/pos2transform.m' property 'autosync' set on 'private/pos2transform.m' property 'autosync' set on 'utilities/private/pos2transform.m' Finally I committed all changes to the files that are involved in the bug2096-cifti branch on github: mac011> svn commit `cat filelist.txt` Sending fileio/ft_filetype.m Adding fileio/ft_read_cifti.m Sending fileio/ft_read_headshape.m Adding fileio/ft_write_cifti.m Sending fileio/ft_write_headshape.m Adding fileio/private/pos2dim.m Adding fileio/private/pos2transform.m Adding fileio/private/read_nifti2_hdr.m Adding fileio/private/write_nifti2_hdr.m Sending ft_connectivityplot.m Sending ft_sourcewrite.m Sending private/pos2dim.m Adding private/pos2dim3d.m Sending private/pos2transform.m Sending test/test_bug2096.m Sending utilities/ft_datatype.m Sending utilities/ft_datatype_volume.m Sending utilities/private/pos2dim.m Sending utilities/private/pos2transform.m Transmitting file data ................... Committed revision 9798. if all went well, I should now be able to pull in that commit again on my github copy...
Robert Oostenveld - 2014-09-15 10:20:24 +0200
(In reply to Robert Oostenveld from comment #9) the merge back into github went fine, it seems from the comment on https://github.com/fieldtrip/fieldtrip that github even detected that the code originates from oostenveld/bug2096-cifti. Following the merge, the only difference remaining is in the documentation of ft_sourcewrite, which seems to be due to whitespace problems. I will copy that over manually. mac011> cp ../fieldtrip-github-oostenveld/ft_sourcewrite.m . mac011> svn commit ft_sourcewrite.m Sending ft_sourcewrite.m Transmitting file data . Committed revision 9800.
Robert Oostenveld - 2014-09-15 10:43:11 +0200
updated test script mac011> svn commit test/test_bug2096.m Sending test/test_bug2096.m Transmitting file data . Committed revision 9802.
Jan-Mathijs Schoffelen - 2014-09-15 10:58:29 +0200
ft_filetype now started returning 'unknown' when inputting a regular nifti file: ft_filetype('somefilenametoananatomicalmri.nii') ans = unknown Could your latest commit have anything to do with this?
Robert Oostenveld - 2014-09-15 14:03:50 +0200
(In reply to Jan-Mathijs Schoffelen from comment #12) this is now fixed.
Diego Lozano Soldevilla - 2014-09-22 12:08:33 +0200
(In reply to Robert Oostenveld from comment #13) Hi Robert, The new input update in pos2transform private function (now only one input, see )produces errors in some of other functions: bash-4.1$ find -type f -name "*.m" -print0 | xargs -0 grep pos2transform | xargs -0 grep pos2transform | grep -v ':%' grep: ./plotting/private/ft_datatype_volume.m: volume.transform = pos2transform(volume.pos, volume.dim); ./utilities/private/pos2transform.m:function [transform] = pos2transform(pos) ./utilities/private/pos2transform.m:% [transform] = pos2transform(pos, dim) where pos is an ordered list of positions ./utilities/ft_datatype_volume.m: volume.transform = pos2transform(volume.pos, volume.dim); ./utilities/ft_checkdata.m: data.transform = pos2transform(data.pos, data.dim); ./fileio/private/ft_checkdata.m: data.transform = pos2transform(data.pos, data.dim); ./fileio/private/pos2transform.m:function [transform] = pos2transform(pos) ./fileio/private/pos2transform.m:% [transform] = pos2transform(pos, dim) where pos is an ordered list of positions ./fileio/ft_write_cifti.m: source.transform = pos2transform(source.pos); ./private/pos2transform.m:function [transform] = pos2transform(pos) ./private/pos2transform.m:% [transform] = pos2transform(pos, dim) where pos is an ordered list of positions ./ft_sourcewrite.m: source.transform = pos2transform(source.pos); ./ft_sourceplot.m: data.transform = pos2transform(data.pos); Do you think it's safe to update them from two to one input? pos2transform(data.pos, data.dim); -> pos2transform(data.pos);
Robert Oostenveld - 2014-09-22 12:10:26 +0200
(In reply to Diego Lozano Soldevilla from comment #14) Hi Diego, thanks for notifying me. This seems to be an unwanted side effect due to the (relatively large) merge. There should not have been anything functionally different in pos2transform. I will look at it. Robert
Robert Oostenveld - 2014-09-22 12:20:45 +0200
(In reply to Robert Oostenveld from comment #15) mac011> svn commit private/pos2transform.m Sending private/pos2transform.m Transmitting file data . Committed revision 9818. mac011> svn update Updating '.': U utilities/private/pos2transform.m U fileio/private/pos2transform.m Updated to revision 9819.
Robert Oostenveld - 2014-09-25 22:42:20 +0200
mac011> svn commit ft_sourceparcellate.m ft_sourcewrite.m fileio/*.m Sending fileio/ft_read_cifti.m Sending fileio/ft_write_cifti.m Sending ft_sourceparcellate.m Sending ft_sourcewrite.m Transmitting file data .... Committed revision 9840.
Robert Oostenveld - 2014-09-25 22:43:55 +0200
I am trying to make an external "stand-alone" copy of the CIFTI read/write functions available at https://github.com/oostenveld/cifti
Robert Oostenveld - 2014-09-27 11:42:26 +0200
mac011> svn commit Sending fileio/ft_read_cifti.m Sending fileio/ft_write_cifti.m Sending ft_sourceparcellate.m Sending ft_sourcewrite.m Sending test/test_bug2096.m Sending utilities/ft_checkdata.m Sending utilities/ft_datatype.m Sending utilities/ft_datatype_parcellation.m Transmitting file data ........ Committed revision 9850. I further finalized and cleaned up the cifti code, this involved removing all old code/comments and general small cleanups to the read and write function. Furthermore, I have been extending the test script, finding and fixing a few bugs on the way. The present implementation appears to be 100% functional, i.e. there are no known issues with it.
Robert Oostenveld - 2014-09-28 18:35:59 +0200
Tim commented: In your *.pscalar.nii: intent_code: 3006 intent_name: ConnParcelScalr The correct intent code is 3008. In your *.ptseries.nii: intent_code: 3000 intent_name: ConnParcelSries The correct intent code is 3004.
Robert Oostenveld - 2014-10-16 18:01:30 +0200
although initially it all seems to work, various improvements have been suggested over the last few days. 1) improve speed of parsing xml, this needs some profiling 2) make reading of surface files optional 3) document the input and output structures 4) improve feedback that the functions are printing on screen (should help the user in debugging)
Robert Oostenveld - 2014-10-17 13:31:48 +0200
more bugs On 17 Oct 2014, at 09:31, Marquand, A.F. <a.f.marquand@fcdonders.ru.nl> wrote: > The first (relatively minor) issue is that ft_write_cifti expects a > field called 'unit', which is not provided by ft_read_cifti. This should be added by the code. It is mm per definition. > Although the cifti is written to disk, I am then unable to open it > in workbench. The error I get in this case is: > > Cifti XML error: index 29696 is not assigned to any model > > Note that I am not modifying the Cifti object in between, it is simply > a load then save operation. That seems more serious, in the sense that some indexing appears to be failing.
Robert Oostenveld - 2014-10-17 13:33:12 +0200
(In reply to Robert Oostenveld from comment #22) this fixes the unit=mm mac011> svn commit fileio/ft_read_cifti.m Sending fileio/ft_read_cifti.m Transmitting file data . Committed revision 9907.
Robert Oostenveld - 2014-10-17 14:42:01 +0200
(In reply to Robert Oostenveld from comment #22) the indexing is addressed in bug 2741.
Andre Marquand - 2014-10-17 16:31:05 +0200
Thanks for sorting those problems Robert. I have a couple of other enhancement requests: - ft_write_cifti currently does not check whether the filename extension is already present in the filename. If the full name is supplied (e.g. xxx.dtseries.nii), the surface filenames are created correctly, but the extension is appended again on line 158 (resulting in e.g. xxx.dtseries.dtseries.nii). Could be an idea to put a check in for this. - Is it possible to add a switch to disable the writing of the surfaces every time? For most of the tasks I need the routines for, writing the surfaces out every time the data changes is not necessary.
Robert Oostenveld - 2014-10-19 11:30:17 +0200
mac011> svn commit fileio/ ft_sourcewrite.m test/test_bug2096.m Sending fileio/ft_read_cifti.m Sending fileio/ft_write_cifti.m Sending ft_sourcewrite.m Sending test/test_bug2096.m Transmitting file data .... Committed revision 9919. I made reading/writing of surface files optional, made debug.xml file optional, changed how the cifti output filename is constructed (parameter is not part of it any more) I also changed the checking of the nii file extension and the dtseries/etc extension.
Robert Oostenveld - 2015-05-08 10:12:00 +0200
I came across two regression errors, and fixed them. mac011> svn commit utilities/private/unparcellate.m Sending utilities/private/unparcellate.m Transmitting file data . Committed revision 10390. mac011> svn commit test/test_bug2096.m Sending test/test_bug2096.m Transmitting file data . Committed revision 10392.
Robert Oostenveld - 2015-06-11 23:17:59 +0200
let me close this bug. The implementation works and is used by quite some people. There are still some issues and suggestions for improvements, but those are better dealt with separately.