Back to the main page.
Bug 2811 - Problem with reading sfp file with headshape points
| Status | CLOSED FIXED |
| Reported | 2015-01-22 14:14:00 +0100 |
| Modified | 2016-05-06 08:04:43 +0200 |
| Product: | FieldTrip |
| Component: | fileio |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Windows |
| Importance: | P5 normal |
| Assigned to: | Jan-Mathijs Schoffelen |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: |
Vladimir Litvak - 2015-01-22 14:14:24 +0100
Created attachment 693 Example sfp file In SPM we use sfp files where 'headshape' label indicates a headshape point. So there can be many points with identical label in the same file (as long as this label is 'headshape'). This worked fine before but now I got an example of a file in which there are over 2000 headshape points and when trying to read it ft_read_headshape gets stuck and I need to crash Matlab. This comes down to line 329 of ft_datatype_sens and line 322 of channelposition. Basically what happens there is that innocent order comparison is done with match_str but as a result each label gets replicated hundreds of thousands of times and then the code gets stuck when trying to go over those labels. Perhaps the fix should be as deep as match_str to make it handle the case of non-unique labels more gracefully as this can also affect other peaces of code. I attach the example file. Vladimir
Jan-Mathijs Schoffelen - 2016-02-21 18:09:53 +0100
Hi Vladimir, I am doing a (non-exhaustive) sweep across all open bugs, to see which ones have become so stale that they can be closed. What's the status of this? Do you have a workaround? Do we need to look into this or can it be closed? Cheers, JM
Vladimir Litvak - 2016-02-21 19:02:53 +0100
I don't think this was ever fixed. Try doing ft_read_sens on the example file. Vladimir
Jan-Mathijs Schoffelen - 2016-02-21 19:14:58 +0100
I did. and indeed it's horrible. It sounds as if a fix is still appreciated, so I'll leave this one open for now...
Jan-Mathijs Schoffelen - 2016-02-21 19:30:29 +0100
As an alternative to Vladimir's suggestion, to go deep into match_str and see whether there's a generic way to solve sheer endless loops when there are many occurrences of the same string, one could think of explicitly handling besa_sfp files as a case in ft_read_headshape.
Jan-Mathijs Schoffelen - 2016-02-21 20:29:32 +0100
I pushed a branch 'bug2811' to my fieldtrip repo on github: github.com/schoffelen/fieldtrip I added a new (dumb) function read_besa_sfp that optionally outputs only the labels that occur only once (useful for reading electrodes, and thus used in ft_read_sens), or optionally outputs all labels (also the multiple occurring 'head shape' labels. Furthermore, I added a case in ft_read_headshape directly dealing with besa_sfp files. Go and check it out! (literally :o)).
Vladimir Litvak - 2016-02-22 16:10:35 +0100
So how does it work now, everyone has their own branch of FT with different fixes? I'd just prefer that this fix propagates to SPM the old way and then I can test it and update our code. Vladimir
Robert Oostenveld - 2016-02-22 16:39:11 +0100
(In reply to Vladimir Litvak from comment #6) Hi Vladimir, let me answer and also CC Guillaume on this Q/A. We have a master branch (shared between the many copies of fieldtrip on github). Most development happens in branches, which (after review and testing) then get merged in the master. The branches are not in the fieldtrip/fieldtrip copy, but for example in the robertoostenveld/fieldtrip or schoffelen/fieldtrip copy. We can look along (or pull) to new code in each other branches, without going through master. Once everyone involved is happy, we merge the branch into fieldtrip/fieldtrip/master. If a change is really trivial, I also do small commits directly to the master at fieldtrip/fieldtrip, but I prefer to use branches as soon as discussion and/or testing is needed. My hope is that master becomes more stable. SPM would continue to use master if it is up to me. I can also imagine (although now it is not concretely planned yet) that we will tag specific points along the fieldtrip master with a version number and that those would become official releases to be included in SPM. So right now master at fieldtrip/fieldtrip serves the same purpose as HEAD used to do in SVN, except that we take a bit more time to think before making changes in it.
Vladimir Litvak - 2016-02-22 16:46:09 +0100
(In reply to Robert Oostenveld from comment #7) Hi Robert, What about if I want to commit a fix to FT? Should I have my own branch as well and commit there and you will decide yourself when to pull it into the main branch? I'd prefer to use SVN interface to github as I do for my DAiSS toolbox. Vladimir
Robert Oostenveld - 2016-02-22 16:57:06 +0100
(In reply to Vladimir Litvak from comment #8) you would make a "fork" of https://github.com/fieldtrip/fieldtrip to https://github.com/vlitvak/fieldtrip. In that fork there would only be a master branch. You could work on your master branch using svn and therefore also make a pull request for your master. To keep your fork up to date with the ongoing changes on the master branch on fieldtrip/fieldtrip, you could use this http://www.hpique.com/2013/09/updating-a-fork-directly-from-github.
Jan-Mathijs Schoffelen - 2016-02-24 09:46:05 +0100
I merged the bug2811 branch into head, since the changes will not affect the grand scheme of things.
Vladimir Litvak - 2016-02-24 11:26:38 +0100
(In reply to Jan-Mathijs Schoffelen from comment #10) Thanks, I'll test. Wouldn't it make sense to grant people who had write access to SVN before (or at least some of them) the permission to update the main branch? If I find a bug now that I can easily fix, what is the sequence of steps to get the fix to propagate to SPM via FT? Should we have a separate branch just for SPM where we'll check in our fixes? I don't mind it if you want to approve everything now but you might be turning yourselves into a bottleneck unnecessarily. I personally only check in things I'm quite confident about. Otherwise I send them to you anyway. Now it could be more convenient to do it as a pull request, but the option to bypass that stage would also be useful. I don't have much experience with git but my concern is that this system motivates forking the code but doesn't really motivate merging it back. Vladimir