Back to the main page.

Bug 2828 - improve support for running FieldTrip with GNU Octave

Status ASSIGNED
Reported 2015-02-05 15:35:00 +0100
Modified 2015-06-22 18:57:12 +0200
Product: FieldTrip
Component: octave
Version: unspecified
Hardware: Macintosh
Operating System: Mac OS
Importance: P5 normal
Assigned to: Robert Oostenveld
URL:
Tags:
Depends on:
Blocks:
See also:

nno - 2015-02-05 15:35:34 +0100

I sent a PR [1] on github; the description is provided here: This is an attempt to improve Fieldtrip compatibitliy [1] with GNU Octave. This PR is far from complete (many things still do not work in Octave, such as several GUI/plotting routines), but is a start to make some basic functionality working (such as ft_defaults). This PR adds: - a function ft_isoctave that returns true if the platform is GNU Octave. - updates to several ft_* functions that relied on matlab-specific constructions in the language. [1] https://github.com/fieldtrip/fieldtrip/pull/54


Robert Oostenveld - 2015-05-04 09:26:31 +0200

rather than making a separate ft_octaveversion, it should be merged in the existing version checking mac011> grep -l matlabversion *.m */*.m ft_analysispipeline.m ft_compile_mex.m ft_databrowser.m ft_defaults.m engine/enginecellfun.m fileio/ft_read_event.m peer/peercellfun.m peer/peerslave.m plotting/ft_select_range.m private/matlabversion.m private/mxSerialize.m qsub/qsubcellfun.m qsub/qsubfeval.m src/mxSerialize.m


Robert Oostenveld - 2015-05-04 09:33:43 +0200

should the handling of functions that are not compatible between matlab and octave be scattered throughout the code, or should we have wrapper functions that switch behaviour depending on where they run? E.g. https://github.com/nno/fieldtrip/commit/8f47a1bc0286174e9bd3b15ac821cd58ee3f0266 is one occurrence of such an incompatibility, which takes 4 lines. The same (similar) lines are needed for all other occurrences. Although for this specific example the same might not happen. I know that in biosig there are (were) a set of functions that replicate MATLAB functions, to allow octave to use the same code. It subsequently turned out that the ones from biosig were not 100% compatible with the MATLAB counterparts, causing incompatibility errors. I solved this by including fieldtrip/external/biosig and having the biosig replacements of standard MATLAB functions in fieldtrip/external/biosig/private.


Robert Oostenveld - 2015-05-04 09:35:08 +0200

The change https://github.com/nno/fieldtrip/commit/3780f3d797937102394ef31ed14e73b1bb66cfeb should also apply to the other drop-in-replacement toolboxes, e.g.fieldtrip/external/signal


nno - 2015-05-04 11:34:55 +0200

Re [1] "rather than making a separate ft_octaveversion, it should be merged in the existing version checking": What would be the best way to do this? 1) changing private/matlabversion.m to support the Octave platform seems not appropriate, because on Octave there is no matlab. Alternatively the function can be renamed. 2) the functions that call matlabversion use it in quite different ways. * ft_defaults: see if functions not present in older versions have to be provided through compat/matlablt2012a * engine/enginecellfun and peer/peercellfun: use onCleanup if available * fileio/ft_read_event: some bit operations depending on the Matlab version * plotting/ft_select_range: use figure handles differently * private/mxSerialize and src/mxSerialize: use either c or c++ implementation for serialisation * qsub/qsubfeval: finds matlab executable depending on version I don't see how this can easily be merged with existing version checking. Different Octave versions also have different capabilities. One solution may be a separate function that indicates whether the current platform provides a certain capability. For example, ft_platform_provides(...), where the argument could be 'onCleanup', 'ui.Figure', 'serialise_cpp', etc. This would be easily extendible if other capability checks would be required. Please let me know if you think this would work, or if you have another solution in mind. [1] http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=2828#c1


nno - 2015-05-04 13:33:20 +0200

Re [1] "The change [to add the stats toolbox only when on Matlab] should also apply to the other drop-in-replacement toolboxes, e.g.fieldtrip/external/signal": Actually the stats and signal toolbox are provided in Octave as external packages. On Matlab, it is my understanding that FieldTrip's ft_defaults overrides the path so that always the functions in external/stats and external/signal are used, even if the statistics toolbox is present. Is that correct? If so, then it may be desirable to have the same behaviour for Octave. On the other hand, if it is desired to add the external/stats and external/signal directories to the path *only* when the functions are not available (e.g. Matlab without external toolboxes), then we may need a function (such as "ft_platform_provides" [2]) to see if the present platform supports stats / signal processing functions. So my question is, what is desirable behaviour for FieldTrip with respect to present or absent Matlab / Octave toolboxes? Always use FT's functions, or only use those when they are not present elsewhere? [1] http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=2828#c3 [2] http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=2828#c4


nno - 2015-05-04 13:38:47 +0200

Re [1] "should the handling of functions that are not compatible between matlab and octave be scattered throughout the code, or should we have wrapper functions that switch behaviour depending on where they run?" Generally, I would think it is preferable to have wrapper functions that show the *same* behaviour irrespective on where they run. (The particular example could actually be simplified and does not seem to require the check for Octave; I updated the PR on github [2]) [1] http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=2828#c2 [2] https://github.com/fieldtrip/fieldtrip/pull/54


nno - 2015-06-17 15:21:42 +0200

I sent a PR [1] on github that aims to address Roberts' comment "rather than making a separate ft_octaveversion, it should be merged in the existing version checking" https://github.com/fieldtrip/fieldtrip/pull/59


nno - 2015-06-17 16:12:56 +0200

To make the proposed changes more modular, I've split them up in several PRs: https://github.com/fieldtrip/fieldtrip/pull/59 https://github.com/fieldtrip/fieldtrip/pull/60 https://github.com/fieldtrip/fieldtrip/pull/61 https://github.com/fieldtrip/fieldtrip/pull/62 Note that #62 is dependent on #59, so it may be better to consider the latter first.


nno - 2015-06-18 11:15:38 +0200

I've also added a PR to read channel layouts: https://github.com/fieldtrip/fieldtrip/pull/63


Robert Oostenveld - 2015-06-18 21:59:06 +0200

I have integrated the change from matlabversion to ft_platform_supports in SVN. This is discussed in more detail on https://github.com/fieldtrip/fieldtrip/pull/59 Sending engine/enginecellfun.m Sending engine/private/fexec.m Adding engine/private/ft_platform_supports.m Deleting engine/private/matlabversion.m Sending fileio/ft_read_event.m Adding fileio/private/ft_platform_supports.m Deleting fileio/private/matlabversion.m Sending fileio/private/mxSerialize.m Sending ft_databrowser.m Sending ft_defaults.m Sending peer/peercellfun.m Sending peer/peerslave.m Sending peer/private/fexec.m Adding peer/private/ft_platform_supports.m Deleting peer/private/matlabversion.m Sending plotting/ft_select_range.m Adding plotting/private/ft_platform_supports.m Deleting plotting/private/matlabversion.m Adding private/ft_platform_supports.m Deleting private/matlabversion.m Sending private/mxSerialize.m Sending qsub/private/fexec.m Adding qsub/private/ft_platform_supports.m Deleting qsub/private/matlabversion.m Sending qsub/qsubcellfun.m Sending qsub/qsubfeval.m Sending src/mxSerialize.m Adding test/private/ft_platform_supports.m Deleting test/private/matlabversion.m Sending test/private/randomseed.m Sending utilities/ft_compile_mex.m Sending utilities/ft_preamble.m Sending utilities/ft_trackusage.m Sending utilities/ft_version.m Adding utilities/private/ft_platform_supports.m Deleting utilities/private/matlabversion.m Sending utilities/private/mxSerialize.m Sending utilities/private/randomseed.m Transmitting file data .............................. Committed revision 10458. I also did roboos@mentat001> svn propset autosync true `find . -name ft_platform_supports.m` and Sending engine/private/ft_platform_supports.m Sending fileio/private/ft_platform_supports.m Sending peer/private/ft_platform_supports.m Sending plotting/private/ft_platform_supports.m Sending private/ft_platform_supports.m Sending qsub/private/ft_platform_supports.m Sending test/private/ft_platform_supports.m Sending utilities/private/ft_platform_supports.m Committed revision 10459.


Robert Oostenveld - 2015-06-18 22:06:12 +0200

(In reply to Robert Oostenveld from comment #10) note that the svn hook takes care of https://twitter.com/fieldtriptoolbx https://code.google.com/p/fieldtrip/source/list https://github.com/fieldtrip/fieldtrip and kicks off the regression test batch that will appear in an hour or so on http://www.fieldtriptoolbox.org/development/dashboard


nno - 2015-06-19 10:52:37 +0200

(In reply to Robert Oostenveld from comment #10) Thanks for making these changes, however there seems to be a problem now. After pulling cd92b73909f103b6595eb32dcdd1a01d9fbc3ef2 from github.com/fieldtrip/fieldtrip:master, ft_platform_supports crashes because "matlabversion" is undefined. For example, the following expression gives an error: ft_platform_supports('onCleanup') This fails because ft_platform_supports uses the functionality in "matlabversion", but this file was deleted in 0939bf90f456ece35b4a96b1f09bdb049382f3ad. To fix this, I see the following options: - matlabversion.m is be added back. - matlabversion is added as a sub function in ft_platform_supports.


nno - 2015-06-19 11:03:01 +0200

(In reply to nno from comment #12) Bug report regarding missing "matlabversion" function here: http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=2911


nno - 2015-06-19 15:56:12 +0200

(In reply to nno from comment #13) The missing matlabversion issue seems fixed in 75f84b74bbe3dc9fe66a295c4326add7ba5a714a.


nno - 2015-06-19 15:57:33 +0200

The following PRs have been updated to reflect the use of ft_platform_supports: https://github.com/fieldtrip/fieldtrip/pull/60 https://github.com/fieldtrip/fieldtrip/pull/61 https://github.com/fieldtrip/fieldtrip/pull/62 https://github.com/fieldtrip/fieldtrip/pull/63 https://github.com/fieldtrip/fieldtrip/pull/64


Robert Oostenveld - 2015-06-22 18:41:32 +0200

the handling of setDefaultStream and setGlobalStream was swapped due to commit 10458 mac011> svn commit qsub/ Sending qsub/private/fexec.m Sending qsub/private/ft_platform_supports.m Transmitting file data .. Committed revision 10465. See also https://code.google.com/p/fieldtrip/source/detail?r=10465


Robert Oostenveld - 2015-06-22 18:57:12 +0200

mac011> svn commit Adding private Adding private/size_equal.m Deleting size_equal.m Transmitting file data . Committed revision 10468. enhancement - allow more than two input arguments, hide size_equal from general users by putting it in private. It is only used here as subfunction, it does not replace a Mathworks stats function. this resolves https://github.com/fieldtrip/fieldtrip/pull/60