Back to the main page.

Bug 1178 - ft_channelcombination fails with 3rd input argument

Status CLOSED WONTFIX
Reported 2011-11-23 09:05:00 +0100
Modified 2016-05-09 08:50:34 +0200
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Mac OS
Importance: P4 minor
Assigned to: Jan-Mathijs Schoffelen
URL:
Tags:
Depends on:
Blocks:
See also:

Robert Oostenveld - 2011-11-23 09:05:40 +0100

if I do the following datachannel = 'a1' 'a2' 'a3' 'b1' 'b2' 'b3' >> ft_channelcombination({'all', 'b*'}, datachannel, 1) ans = 'a1' 'b1' 'a2' 'b1' 'a3' 'b1' 'b2' 'b1' 'b3' 'b1' 'a1' 'b2' 'a2' 'b2' 'a3' 'b2' 'b3' 'b2' 'a1' 'b3' 'a2' 'b3' 'a3' 'b3' 'a1' 'a1' 'a2' 'a2' 'a3' 'a3' 'b1' 'b1' 'b2' 'b2' 'b3' 'b3' then I would not expect the combinations that end with a1, a2, a3. My expectation is that autocombinations are kept within the set, and not that they are added outside the scope of the selected set.


Jan-Mathijs Schoffelen - 2011-11-23 15:48:57 +0100

why would this matter?


Jan-Mathijs Schoffelen - 2011-11-23 15:51:01 +0100

Oh I see the point, I thought it was about the order of the channel pairs. But you mean that the a's should not be in the output. From the functionality of the function it makes sense, yet if one wants to compute coherence later on, then the auto-combinations for the 'a'-channels still need to be there.


Boris Reuderink - 2011-11-28 15:42:16 +0100

I share Roberts expectations, but also JM's point seems fair. What would be the best solution? Change the function's help to explain its behavior, or change the behavior? Related, the help string contains the following sentence: "Please note that the default behaviour is to exclude to exclude symetric pairs and auto-combinations." To me it suggests the behavior Robert expected, and I think the sentence needs some attention.


Robert Oostenveld - 2011-11-29 10:25:35 +0100

if you represent it as matrix with "from-to" connections, then the default behavior is to return the upper triangle (or the lower one?). The auto-connections are on the diagonal, the lower (or upper?) ones can also be returned. To me it makes sense to have two options, whether you want unidirectional (only upper) or bidirectional (upper+lower triangle), and whether you want auto (diagonal) combinations or not. So there are 2x2 options: - upper - upper+lower - upper+auto - upper+lower+auto Based on these options, I think the users interface to the function could also be unambiguous.


Boris Reuderink - 2012-01-03 12:36:42 +0100

Clear description :). Changed status to new.


Jan-Mathijs Schoffelen - 2012-01-27 08:45:58 +0100

assigned bug to get all bugs lined up before the grand bug binge


Jan-Mathijs Schoffelen - 2015-08-07 14:56:24 +0200

Rather than upper and lower, I think that, if anything, the distinction should be 'column' and 'row', because this describes the inflow and outflow. A given element on the lower triangular part can either be in or outflow. a grep on ft_channelcombination yields: ft_connectivityanalysis ft_freqanalysis ft_mvaranalysis Only ft_connectivityanalysis calls it with the 3d input argument. The other two call it the old-fashioned way. Frankly, at this moment in time I don't think it worthwile to pursue this.


Jan-Mathijs Schoffelen - 2015-09-08 16:51:52 +0200

Following up on this: I made a new implementation, which I think is cleaner. It now allows for two optional additional inputs, where the first one determines the inclusion of the auto-combinations, and the second one determines the upper/lower/both triangle behavior. According to the test function, the default behavior (i.e. without addition input arguments) mirrors the old functionality (which I copied over into the function body of test_ft_channelcombination).


Jan-Mathijs Schoffelen - 2015-09-08 16:53:20 +0200

committed to SVN-repository revision 10654