Back to the main page.

Bug 1646 - ft_prepare_mesh_new needs to be integrated with ft_prepare_mesh

Status CLOSED FIXED
Reported 2012-08-10 17:52:00 +0200
Modified 2013-01-16 15:07:18 +0100
Product: FieldTrip
Component: forward
Version: unspecified
Hardware: PC
Operating System: Windows
Importance: P3 normal
Assigned to: Johanna
URL:
Tags:
Depends on:
Blocks:
See also: http://bugzilla.fcdonders.nl/show_bug.cgi?id=105http://bugzilla.fcdonders.nl/show_bug.cgi?id=115http://bugzilla.fcdonders.nl/show_bug.cgi?id=1651http://bugzilla.fcdonders.nl/show_bug.cgi?id=222http://bugzilla.fcdonders.nl/show_bug.cgi?id=937http://bugzilla.fcdonders.nl/show_bug.cgi?id=964http://bugzilla.fcdonders.nl/show_bug.cgi?id=997

Lilla Magyari - 2012-08-10 17:52:07 +0200

hi, there are two functions which are quite similar (but not the same): ft_prepare_mesh and ft_prepare_mesh_new. Am I right that this later one should be called ft_prepare_mesh and ft_prepare_mesh_new should not exist? I think the documentation of these functions should be also updated, because they refer to ft_prepare_bemmodel... etc. functions, instead of ft_prepare_headmodel. And what does cfg.sourceunit do? Because this function is also used for creating a (BEM) headmodel, therefore, the name sourceunit is confusing here. And it has to be specified explicitly, even if the input (for example, the segmented mri) has a unit field. Lilla


Robert Oostenveld - 2012-08-13 15:34:21 +0200

see http://fieldtrip.fcdonders.nl/development/fwdarch http://code.google.com/p/fieldtrip/source/browse/#svn%2Fbranches%2Fsurface


Robert Oostenveld - 2012-09-27 14:10:22 +0200

Johanna and I are going to work on it together.


Robert Oostenveld - 2012-09-27 17:40:31 +0200

how to deal with cfg.tissue? If the input is an indexed representation and cfg.tissue is equal to unique(mri.seg(:)) then there is no problem. If the input is an indexed representation and cfg.tissue is empty, then cfg.tissue should be set to unique(mri.seg(:)) If the input is a tpm representation then cfg.tissue should be a string pointing to the field.


Robert Oostenveld - 2012-09-28 11:55:08 +0200

TODO: the test script for bug 937 needs to be revisited, it has some cases in it which are not desired any more. Furthermore, it calls ft_prepare_mesh_new rather than ft_prepare_mesh.


Robert Oostenveld - 2012-09-28 12:55:48 +0200

manzana> svn commit Sending forward/ft_convert_units.m Sending forward/ft_headmodel_bemcp.m Sending ft_prepare_mesh.m Deleting ft_prepare_mesh_new.m Sending private/prepare_mesh_segmentation.m Deleting private/prepare_mesh_segmentation_new.m Adding test/test_bug1646.m Sending utilities/ft_checkdata.m Sending utilities/ft_datatype.m Sending utilities/ft_datatype_segmentation.m Transmitting file data ........ Committed revision 6575.


Johanna - 2012-09-28 18:06:39 +0200

Re: comment 4: bug 937 dealt with, aside from remaining question raised inside that bug. Re: comment 3: "If the input is an indexed representation and cfg.tissue is equal to unique(mri.seg(:)) then there is no problem." I think it's meant that if cfg.tissue=setdiff(unique(mri.seg(:)),0) then should work with indexed representation. Tested/works now with seg5. "If the input is an indexed representation and cfg.tissue is empty, then cfg.tissue should be set to unique(mri.seg(:))" cfg.tissue empty because user-specified cfg.tissue=[], or because user did not specify cfg.tissue at all? In current code: if basedonseg or basedonmri, then cfg.tissue is required field, but [] is allowed via ft_getopt(cfg,'tissue'). Could we update so to remove ft_getopt(cfg,'tissue'), as it not needed, and then ft_checkconfig(cfg, 'required', {'tissue', 'numvertices'}) will crash if user doesn't at least make it empty input? Related, I think it makes sense to move some of the extra checks on relating cfg.tissue and cfg.numvertices to each other, to within prepare_mesh_segmenatation, as they are only needed in there, and thus should be available for any other fcn calling prepare_mesh_segmentation. "If the input is a tpm representation then cfg.tissue should be a string pointing to the field." (will deal with this later...)


Johanna - 2012-10-02 10:49:09 +0200

Hi Robert, after the updates with convert_segmentation subfunctions yesterday, a few things were crashing in a new way. I traced it to ft_datatype_segmentation if probabilistic fixsegmentation(segmentation, fn, 'probabilistic'); elseif indexed fixsegmentation(segmentation, fn, 'indexed'); end should instead be: if probabilistic segmentation = fixsegmentation(segmentation, fn, 'probabilistic'); elseif indexed segmentation = fixsegmentation(segmentation, fn, 'indexed'); end fixed svn 6626.


Robert Oostenveld - 2012-10-02 11:01:07 +0200

(In reply to comment #7) i made the corresponding change to the parcellation version manzana> svn commit Sending utilities/ft_datatype_parcellation.m Transmitting file data . Committed revision 6627.


Johanna - 2012-10-02 13:17:23 +0200

I resolved "If the input is an indexed representation and cfg.tissue is empty, then cfg.tissue should be set to unique(mri.seg(:))" by letting cfg.tissue=[] or no cfg.tissue specificiation both be ok for indexed representation. "If the input is a tpm representation then cfg.tissue should be a string pointing to the field." The one exception is when .gray, .white .csf exist and the user calls cfg.tissue='brain', then the code can handle this. Otherwise I have added an explanatory error message if the user gives a cfg.tissue='randomstring' that doesn't correspond to a tpm field in the segmentation. And finally, a cfg.tissue=1 should be allowed for tpm, thus it need not be a string. (but new error is given if cfg.tissue=4 and only 3 tpm's in the segmentation, for example). svn commit 6630. closing bug.


Roemer van der Meij - 2012-10-10 15:31:09 +0200

Hi Johanna, I opened this bug again during the bug binge, because test_bug1646 fails on line 167, with the error being produced by ft_prepare_mesh on line 138. There's a call to checkconfig done there, which spouts out an error if cfg.tissue doesn't exist or has a different value. It's not given as input to the test script. If this is desired behaviour, could you make a try with a catch for the specific error message? (I cannot oversee the prepare_mesh stuff) try ft_prepare_mesh catch err if strcmp(err, 'The field cfg.tissue is required') % succes else error('test doesn''t result in expected error message') end err is automatically saved in workspace, or something similar, just found out :)


Johanna - 2012-10-19 12:45:15 +0200

fixed ft_prepare_mesh.m, r6771. test_bug1646 now runs.