Back to the main page.
Bug 1722 - why is cfg.warp only used if cfg.method=template, in ft_electroderealign?
Status | REOPENED |
Reported | 2012-09-19 17:16:00 +0200 |
Modified | 2015-02-12 18:20:47 +0100 |
Product: | FieldTrip |
Component: | core |
Version: | unspecified |
Hardware: | PC |
Operating System: | Windows |
Importance: | P3 normal |
Assigned to: | Johanna |
URL: | |
Tags: | |
Depends on: | |
Blocks: | |
See also: |
Johanna - 2012-09-19 17:16:24 +0200
currently starting line 477 switch cfg.method case 'template' norm.chanpos = warp_apply(norm.m, orig.chanpos, cfg.warp); case {'fiducial' 'interactive'} norm.chanpos = warp_apply(norm.m, orig.chanpos); case 'manual' % the positions are already assigned in correspondence with the mesh norm = orig; otherwise error('unknown method'); end But why not do this? Is there a reason why cfg.warp shouldn't be used with cfg.method='fidicial' or 'interactive'? switch cfg.method case 'template' norm.chanpos = warp_apply(norm.m, orig.chanpos, cfg.warp); case {'fiducial' 'interactive'} switch cfg.warp case 'rigidbody' norm.chanpos = warp_apply(norm.m, orig.chanpos); otherwise norm.chanpos = warp_apply(norm.m, orig.chanpos, cfg.warp); end case 'manual' % the positions are already assigned in correspondence with the mesh norm = orig; otherwise error('unknown method'); end
Robert Oostenveld - 2012-09-19 17:37:14 +0200
I think that method=fiducial implies that warp=rigidbody (i.e. 6 params), and that method=interactive implies that warp=traditional (i.e. 9 params).
Robert Oostenveld - 2012-09-19 17:37:56 +0200
(In reply to comment #1) If that were the case, it would be good to set it as default, or if otherwise specified by the user to give an error.
Johanna - 2012-09-19 18:09:23 +0200
I agree good to warn user if they specified a cfg.warp other than what is being used. However, the reason I ran into this was that I wanted to use method=interactive, but also want to use warp=globalscale as there seems to be an inward shift of Polhemus values (where I got the electrode information from) (but warp=traditional would also work for me, but I don't think that's happening). Thus, should I force warp=traditional if method=interactive? or only if user specifies it, otherwise use default warp=rigidbody?
Robert Oostenveld - 2012-11-08 18:01:03 +0100
I suggest that for all instances of y2 = warp_apply(x, y1, z) that input parameter z is explicitly specified. There are now these three cases mac001> grep -n warp_apply ft_electroderealign.m 389: norm.chanpos = warp_apply(norm.m, elec.chanpos, 'homogeneous'); 479: norm.chanpos = warp_apply(norm.m, orig.chanpos, cfg.warp); 481: norm.chanpos = warp_apply(norm.m, orig.chanpos); I suggest to change the first and third into if ~strcmp(cfg.warp, 'homogenous') % or something else for the 3rd error(... give explanation ...); else % just do it norm.chanpos = warp_apply(norm.m, elec.chanpos, cfg.warp); end
Johanna - 2015-02-04 19:03:50 +0100
I've rediscovered this bug and so will now actually fix it (hey, better late than never). zumerj@psychl-132432-1:~/home/fieldtrip_svn$ svn commit ft_electroderealign.m Sending ft_electroderealign.m Transmitting file data . Committed revision 10163.
Johanna - 2015-02-05 12:30:55 +0100
oops, I created a bug with this change. turns out that norm.chanpos = ft_warp_apply(norm.m, orig.chanpos); and norm.chanpos = ft_warp_apply(norm.m, orig.chanpos, 'traditional'); give very different results, for the same norm.m of [1 0 0 0 0 1 0 0 0 0 1 0 0 0 0 1] I reverted the change to the way without the third argument, but this should be figured out why the result is unexpected. zumerj@psychl-132432-1:~/home/fieldtrip_svn$ svn commit ft_electroderealign.m Sending ft_electroderealign.m Transmitting file data . Committed revision 10165.