duneuro integration & files organisation

[tmedani]

Hey @ftadel & @juangpc,

Sorry if you are getting many spams due to my recent activities in this repo. I'm still learning how to use git and its fabulous features.

So to keep all the tracks on the code, this repo is connected to another repo: (I discovered this funny git option 'submodule')

@ftadel here is the invitation, it is a toolbox called "duneuro interface", where I collected the main functions to run and test duneuro from matlab.

@ftadel I followed your recommendation Nb 2, and I uploaded all to the bst_duneuro toolbox. At the end of this message, tell me if I need to apply the recommendation Nb 1.

all the functions related to brainstorm integration are, hopefully, in the folder bst-duneuro. otherwise, they are somewhere in the other folder named "toolbox". We need just to move them to "bst-duneuro" folder.

Then, in this repo, I added a temporary folder, "bst_addons", where I put the original functions of brainstorm that I modified, listed in this picture (this image is also in this folder)

With all these together, I was able to run the EEG, MEG and combined MEG/EEG.

Also, please, don't hesitate to change, comment, criticize, and of course, modify these functions. this is just a first attempt to make duneuro working from brainstorm.

I have still a lot of improvement that I notified on the code, but you know, code optimization has no limit.

@ftadel I added a lot of comments on the code, as a TODO, where either I didn't know how to do it, or the way how I did it is not the best one.

If you want, we can also plan a bluejeans/skype discussion in order to clarify all these.

Thank you & A+ Takfarinas

[ftadel]

I discovered this funny git option 'submodule'

Funny, maybe, but it doesn't look that easy to manipulate... I'll let you deal with this fancy feature and will only work with the repo brainstorm-tools/bst-duneuro :)

tell me if I need to apply the recommendation Nb 1.

Yes, delete this subfolder bst_addons from this repo and open a PR to push them to the brainstorm3 repo.

otherwise, they are somewhere in the other folder named "toolbox".

Can you make sure it works without any functions outside of brainstorm3 and bst-duneuro folders? (just remove the extra folders from your Matlab path)

If you want, we can also plan a bluejeans/skype discussion in order to clarify all these.

I'll start reviewing all your code after your create the brainstorm3 repo pull request, and we'll see if we need to schedule some more live discussions. (not sure I'll have time to do it before January, but I guess there is no emergency, is there?)

[tmedani]

ok, sounds good, No, there is no emergency.

[juangpc]

Hey, I'm ready to work a little bit on the documentation. Is it ok if I push some changes here?

[tmedani]

Hey, I'm ready to work a little bit on the documentation. Is it ok if I push some changes here?

Everything & any contributions are welcome !!

[juangpc]

Hi,

I've finished the documentation and during the following minutes will push some final versions of the compilation script. I've managed to make it as simple and seamless as possible. If you clone the repo and run

config/setup_bst_duneuro.sh all

It downloads the source code, cleans previous builds (reduces the chances of problems during compilation), builds everything both for windows and Linux and finally copies the binary application files to /bin folder with the compilation date in the name of the app.

@ftadel following your idea, I've checked and run the compilation using a new ubuntu virtualbox instance. So I guess anybody could re-do the compilation again.

Hope this script is useful once we move on and forget how this all worked.

Let me know if you have any doubts.

@tmedani 2 things. Could you check that the cpp files in config/toto folder are the last version of the application? I'm wondering if we could you put all these .m files in the repo's main folder into another folder, like say... matlab or something of the sort? I think it would make it all a bit cleaner. Right now the readme file is only displayed way down after showing all the list of files.

[tmedani]

Hey there,

Thank you, @juangpc for this wonderful work. I would say, this is a nice "compression" of the hard steps in one easy command. I hope also that will be useful, we need to document all this.

Just a small comment, is it possible to change the "toto" folder to "brainstorm_app" os something similar?

@tmedani 2 things. Could you check that the cpp files in config/toto folder are the last version of the application?

I have checked, and update it. The original files are in this repo: https://github.com/tmedani/duneuro_interface/tree/master/toolbox/duneuro_cpp/src

I'm wondering if we could you put all these .m files in the repo's main folder into another folder, like say... matlab or something of the sort? I think it would make it all a bit cleaner. Right now the readme file is only displayed way down after showing all the list of files.

done, all the matalb code is moved to a new folder called "matlab"

[tmedani]

Comments on Matlab and mex file implementation. If you have Matlab installed in your linux version and you plan to use Brainstorm in this system, you can take advantage of the possibility of building the application as a mex file which will decrease execution times and allow for more interaction between Matlab and duneuro.

In order to do this modify Matlab's path in config/config_release_linux.opts and modify src/duneuro-matlab/toto/CMakeLists.txt file in order to add a separate mex application.

@juangpc : for this part, we need to write a new interface that call original Duneuro Matlab binding.

[ftadel]

@juangpc I would be amazing to have something that simple working!

I tried to run your script on my WSL/Ubuntu, but it didn't work. Before the long error log, two minor comments:

• The file config/setup_bst_duneuro.sh is not executable in the repo, easy to fix
• The script doesn't work if we call it from the config folder, it has to be executed from the bst-duneuro folder directly, which is not so intuitive.

Now the error log:

#####################################################

               Patching fortran issue!!

#####################################################

sed: can't read src/dune-common/cmake/modules/DuneMacros.cmake: No such file or directory

Copying toto folder to duneuro-matlab.

cp: cannot stat 'config/toto': No such file or directory
grep: src/duneuro-matlab/CMakeLists.txt: No such file or directory

Adding subdirectory toto to cmake lists file.

sed: can't read src/duneuro-matlab/CMakeLists.txt: No such file or directory
grep: config/config_release_windows.opts: No such file or directory

Adding full path to toolchain file.

sed: can't read config/config_release_windows.opts: No such file or directory

Deleting previous builds.

Building duneuro. Log file created in build_release_windows_log.txt

----- using default flags $CMAKE_FLAGS from /home/ftadel/bst-duneuro/config/config_release_windows.                         opts -----
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 201: pkg-config: command                          not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 392: pkg-config: command                          not found
ERROR: could not find module 'dune-functions',
       module is also unknown to pkg-config.
       Maybe you need to adjust PKG_CONFIG_PATH!
       'dune-functions' is required by dune-pdelab
Execution of dunecontrol terminated due to errors!

 Something went wrong. Check build_release_windows_log.txt or build_release_windows_rebuild_log.txt                          .

Deleting previous builds.

Building duneuro. Log file created in build_release_linux_log.txt

----- using default flags $CMAKE_FLAGS from /home/ftadel/bst-duneuro/config/config_release_linux.op                         ts -----
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 201: pkg-config: command                          not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command n                         ot found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 392: pkg-config: command                          not found
ERROR: could not find module 'dune-functions',
       module is also unknown to pkg-config.
       Maybe you need to adjust PKG_CONFIG_PATH!
       'dune-functions' is required by dune-pdelab
Execution of dunecontrol terminated due to errors!

 Something went wrong. Check build_release_linux_log.txt or build_release_linux_rebuild_log.txt .

[juangpc]

That's new. something not working. I'll fix the issues.

[juangpc]

Hi, the script setup_bst_duneuro.sh :

• has a 777 permission scheme.
• works from wherever you want to call it from.
• it downloads builds and saves into bin/ the versions for windows and linux.

Hope it works.

The output will be a file named bst_duneuro_meeg_<<date>><<ext>> where <> can, for instance, be "24_01_2020" and extension can be ".exe" in windows and empty in linux.

This means that matlab's call to the executable will need to include the date/version number. I got this idea from the way inverse computation scripts are called (2016 vs 2018 versions, etc...). I think we should enforce some kind of way to verify which version it is being called. We have too many links in this chain and if something is not working we should be able to easily discard sources of the error. This is my intention with the builddate-in-the-name thing. Another possibility would be to have the output of the executable application include the version of the code being executed. Right now, the app will create two separate binary files each storing some matrix. The binary information is preseeded with the following info: ::numRows::numCols:: followed by floating point numbers which are subsecuently read with fread from matlab. If you guys think so, I can add some kind of ::version::numrows::numcols:: or something of the sort. But for now, to have at least the day of the build in the name of the file is something. Let me know what you think.

[juangpc]

Ah! and toto->brainstorm_app...

[tmedani]

Super @juangpc ... I will set up a new virtual machine and check it. also, does it make sense to move the file: src_vault.tar.gz to "config" folder

This means that matlab's call to the executable will need to include the date/version number. I got this idea from the way inverse computation scripts are called (2016 vs 2018 versions, etc...). I think we should enforce some kind of way to verify which version it is being called.

I have just added a way to call the exact 'binaries' from Matlab, assuming that the binaries of the linux and windows are the same (https://github.com/brainstorm-tools/bst-duneuro/commit/8476ca72d622cbc017eac63ee26426a4c3625211#r36958908).

This is my intention with the builddate-in-the-name thing. Another possibility would be to have the output of the executable application include the version of the code being executed. Right now, the app will create two separate binary files each storing some matrix. The binary information is preseeded with the following info: ::numRows::numCols:: followed by floating point numbers which are subsecuently read with fread from matlab. If you guys think so, I can add some kind of ::version::numrows::numcols:: or something of the sort. But for now, to have at least the day of the build in the name of the file is something. Let me know what you think.

also good, but need more modification on the Cpp code-writer and the MatLab reader.

[juangpc]

yes. that is actually a nice idea. and if the server is down we can automate the unzip into src. all seamless to the indifferent user.

On Fri, Jan 24, 2020 at 5:36 PM Takfarinas MEDANI notifications@github.com wrote:

Super @juangpc https://github.com/juangpc . does it make sense to move the file: src_vault.tar.gz to "config" folder

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/brainstorm-tools/bst-duneuro/issues/1?email_source=notifications&email_token=ACEKMIER6SEZSJ43LKDPSATQ7N3OXA5CNFSM4J4YVMMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ4NDUI#issuecomment-578343377, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACEKMIAKIAL4DHWUNHSUYUTQ7N3OXANCNFSM4J4YVMMA .

-- Juan

[ftadel]

bst_duneuromeeg<><>

Maybe there are things I didn't follow here, but this does not seem to be a very standard practice, and I'm not sure this is necessary because there is already a check done in Brainstorm itself (and the installation/update of these binaries will be done by Brainstorm in most cases). When using Brainstorm, it will always ask to use the latest package of bst-duneuro available on the neuroimage webserver. The current online version is returned with a PHP script (https://neuroimage.usc.edu/bst/getversion_duneuro.php) and the current version installed on the computer is saved in the user folder ($HOME/.brainstorm/bst-duneuro/url).

I added a webhook that calls a PHP script on the neuroimage server that repackages bst-duneuro.zip every time something is pushed to the bst-duneuro repository. So if you update the binaries on the github, you'll be sure that everybody start using this new version immediately.

[ftadel]

And regarding the compilation, I still have errors like these. Is there anything I'm doing wrong?

ftadel@DESKTOP-SQA66ET:~/bst-duneuro/config$ ./setup_bst_duneuro.sh  all

Deleting previous builds.

Deleting previously downloaded code.

Deleting previously downloaded code.

Cloning into 'dune-common'...
remote: Enumerating objects: 59395, done.
remote: Counting objects: 100% (59395/59395), done.
remote: Compressing objects: 100% (15126/15126), done.
remote: Total 59395 (delta 44144), reused 59340 (delta 44103)
Receiving objects: 100% (59395/59395), 13.63 MiB | 2.06 MiB/s, done.
Resolving deltas: 100% (44144/44144), done.
Checking connectivity... done.
Cloning into 'dune-geometry'...
remote: Enumerating objects: 6246, done.
remote: Counting objects: 100% (6246/6246), done.
remote: Compressing objects: 100% (1646/1646), done.
remote: Total 6246 (delta 4603), reused 6208 (delta 4570)
Receiving objects: 100% (6246/6246), 1.94 MiB | 1.67 MiB/s, done.
Resolving deltas: 100% (4603/4603), done.
Checking connectivity... done.
Cloning into 'dune-grid'...
remote: Enumerating objects: 76122, done.
remote: Counting objects: 100% (76122/76122), done.
remote: Compressing objects: 100% (17340/17340), done.
remote: Total 76122 (delta 58669), reused 76028 (delta 58607)
Receiving objects: 100% (76122/76122), 22.57 MiB | 1.19 MiB/s, done.
Resolving deltas: 100% (58669/58669), done.
Checking connectivity... done.
Checking out files: 100% (545/545), done.
Cloning into 'dune-localfunctions'...
remote: Enumerating objects: 15343, done.
remote: Counting objects: 100% (15343/15343), done.
remote: Compressing objects: 100% (3126/3126), done.
remote: Total 15343 (delta 11923), reused 15263 (delta 11848)
Receiving objects: 100% (15343/15343), 1.99 MiB | 2.28 MiB/s, done.
Resolving deltas: 100% (11923/11923), done.
Checking connectivity... done.
Cloning into 'dune-istl'...
fatal: unable to access 'https://gitlab.dune-project.org/core/dune-istl.git/': Failed to connect to gitlab.dune-project.org port 443: Connection refused
Cloning into 'dune-typetree'...
fatal: unable to access 'https://gitlab.dune-project.org/staging/dune-typetree.git/': Failed to connect to gitlab.dune-project.org port 443: Connection refused
Cloning into 'dune-uggrid'...
fatal: unable to access 'https://gitlab.dune-project.org/staging/dune-uggrid.git/': gnutls_handshake() failed: The TLS connection was non-properly terminated.
Cloning into 'dune-functions'...
remote: Enumerating objects: 9922, done.
remote: Counting objects: 100% (9922/9922), done.
remote: Compressing objects: 100% (3050/3050), done.
remote: Total 9922 (delta 6884), reused 9887 (delta 6860)
Receiving objects: 100% (9922/9922), 1.84 MiB | 1.61 MiB/s, done.
Resolving deltas: 100% (6884/6884), done.
Checking connectivity... done.
Note: checking out 'bd847eb9f6617b116f5d6cb4930e5417d7b6e9a7'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at bd847eb... [doc] Introduce uniform and flat index maps
Switched to a new branch 'c-interface'
Cloning into 'dune-pdelab'...
remote: Enumerating objects: 40262, done.
remote: Counting objects: 100% (40262/40262), done.
remote: Compressing objects: 100% (10318/10318), done.
remote: Total 40262 (delta 29328), reused 40156 (delta 29238)
Receiving objects: 100% (40262/40262), 6.61 MiB | 2.03 MiB/s, done.
Resolving deltas: 100% (29328/29328), done.
Checking connectivity... done.
Checking out files: 100% (420/420), done.
Cloning into 'duneuro'...
remote: Enumerating objects: 3298, done.
remote: Counting objects: 100% (3298/3298), done.
remote: Compressing objects: 100% (1007/1007), done.
remote: Total 3298 (delta 2436), reused 3047 (delta 2268)
Receiving objects: 100% (3298/3298), 916.05 KiB | 0 bytes/s, done.
Resolving deltas: 100% (2436/2436), done.
Checking connectivity... done.
Cloning into 'duneuro-matlab'...
fatal: unable to access 'https://gitlab.dune-project.org/duneuro/duneuro-matlab.git/': gnutls_handshake() failed: The TLS connection was non-properly terminated.

#####################################################

               Patching fortran issue!!

#####################################################

Copying brainstorm_app folder to duneuro-matlab.

Adding subdirectory brainstorm_app to cmake lists file.

Adding full path to toolchain file.

Deleting previous builds.

Building duneuro. Log file created in build_release_windows_log.txt

----- using default flags $CMAKE_FLAGS from /home/ftadel/bst-duneuro/config/config_release_windows.opts -----
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 201: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 392: pkg-config: command not found
WARNING: could not find module 'dune-uggrid',
       module is also unknown to pkg-config.
       Maybe you need to adjust PKG_CONFIG_PATH!
       'dune-uggrid' is suggested by dune-grid
Skipping 'dune-uggrid'!
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 392: pkg-config: command not found
ERROR: could not find module 'dune-typetree',
       module is also unknown to pkg-config.
       Maybe you need to adjust PKG_CONFIG_PATH!
       'dune-typetree' is required by dune-functions
Execution of dunecontrol terminated due to errors!

 Something went wrong. Check build_release_windows_log.txt or build_release_windows_rebuild_log.txt .

Deleting previous builds.

Building duneuro. Log file created in build_release_linux_log.txt

----- using default flags $CMAKE_FLAGS from /home/ftadel/bst-duneuro/config/config_release_linux.opts -----
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 201: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 88: pkg-config: command not found
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 392: pkg-config: command not found
WARNING: could not find module 'dune-uggrid',
       module is also unknown to pkg-config.
       Maybe you need to adjust PKG_CONFIG_PATH!
       'dune-uggrid' is suggested by dune-grid
Skipping 'dune-uggrid'!
/home/ftadel/bst-duneuro/src/dune-common/bin/../lib/dunemodules.lib: line 392: pkg-config: command not found
ERROR: could not find module 'dune-typetree',
       module is also unknown to pkg-config.
       Maybe you need to adjust PKG_CONFIG_PATH!
       'dune-typetree' is required by dune-functions
Execution of dunecontrol terminated due to errors!

 Something went wrong. Check build_release_linux_log.txt or build_release_linux_rebuild_log.txt .

[juangpc]

Hi, @tmedani I've added the feature of automatically unzip the vault file whenever the download process didn't work for any reason. I think it is much cleaner now.

pkg-config: command not found

@ftadel I think your problem is that you don't have installed pkg-config. Please do the following.

sudo apt-get install git pkg-config cmake mingw-w64 g++-mingw-w64 libc6-dev-i386 python3-pip libeigen3-dev

try calling setup_bst_duneuro.sh all from wherever you want to. At the end, you should be able to find two apps in the test/ folder called: bst_duneuro_meeg_<<date>> and bst_duneuro_meeg_<<date>>.exe for linux and win versions.

Sorry for previous not-perfectly working deploys. The last thing I want to do is make you waste your time. I've downloaded everything from scracth and tested it twice and it is working here. Let me know if something's not working again.

@ftadel I've updated the readme file so that the packages needed etc... are more clear now.

[tmedani]

bst_duneuromeeg<><>

Maybe there are things I didn't follow here, but this does not seem to be a very standard practice, and I'm not sure this is necessary because there is already a check done in Brainstorm itself (and the installation/update of these binaries will be done by Brainstorm in most cases). When using Brainstorm, it will always ask to use the latest package of bst-duneuro available on the neuroimage webserver. The current online version is returned with a PHP script (https://neuroimage.usc.edu/bst/getversion_duneuro.php) and the current version installed on the computer is saved in the user folder ($HOME/.brainstorm/bst-duneuro/url).

I added a webhook that calls a PHP script on the neuroimage server that repackages bst-duneuro.zip every time something is pushed to the bst-duneuro repository. So if you update the binaries on the github, you'll be sure that everybody start using this new version immediately.

Ok, then the user will not be aware about the duneuro binaries updates, I think also he doesn't need it. @juan how should we proceed?

I think in any case, from the "coding" view, both versions will work.

[juangpc]

OK @tmedani and @ftadel I see what you mean:

• Production-wise, the webhook captures binaries called bst_duneuro_meeg and bst_duneuro_meeg.exe from bin/ . So far so good.
• Development-wise, the problem is that the development environment is part of duneuro which is updated separately by another repository. So here is my solution:
  1. The source is hard linked between the config/brainstorm_app folder and the src/duneuro-matlab/brainstorm_app. This way it all compiles and code updates are seamlessly updated to github. 2. None of the build* folders nor the logs get pushed to github.
  2. Cpp development code is synced to github and compiled at the same time. Setup script copies the binaries to the test/ folder where tests can be performed. @tmedani can you push all the development ini files to test/ folder in the repository?
  3. Once a new feature is developed tested and so forth, it can be manually copied to bin/ folder as a way to "put it into production". Is this ok with you people? I understand it is a bit messy but is the least messy it can get.

[juangpc]

At some point Id like to try that git large file storage for the zipped src folder.

[juangpc]

Hi, I've been reading about it. git large files support requires all collaborators to have installed the feature, otherwise, they'll just see a reference file not the actual file. Given it is just one file, and that the amount of forks in this repo is going to probably be... let's say, close to zero... I'm dropping my idea of using such a feature of git. Though it is one fine thing to know about.

Pd. I've been trying to do a bit of further testing and GitLab is down right now... Good thing we have this vault.

[tmedani]

2. . @tmedani can you push all the development ini files to test/ folder in the repository?

@juangpc the files are already in the folder

3. Once a new feature is developed tested and so forth, it can be manually copied to bin/ folder as a way to "put it into production". Is this ok with you people? I understand it is a bit messy but is the least messy it can get

sounds good. I think we are not going to change many time these binaries.

[juangpc]

never say never.

On Tue, Jan 28, 2020 at 11:29 AM Takfarinas MEDANI notifications@github.com wrote:

2. . @tmedani https://github.com/tmedani can you push all the development ini files to test/ folder in the repository?

@juangpc https://github.com/juangpc the files are already in the folder

3. Once a new feature is developed tested and so forth, it can be manually copied to bin/ folder as a way to "put it into production". Is this ok with you people? I understand it is a bit messy but is the least messy it can get

sounds good. I think we are not going to change many time these binaries.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/brainstorm-tools/bst-duneuro/issues/1?email_source=notifications&email_token=ACEKMIHBSRRMJZEDFQIO55LRABTPZA5CNFSM4J4YVMMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKEGODQ#issuecomment-579364622, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACEKMIAH2ZT23V5OATT6HTLRABTPZANCNFSM4J4YVMMA .

-- Juan

[tmedani]

Hello,

@ftadel I updated the bst-duneuro folder, it's ready for your reviews. I fully tested a simple EEG Duneuro computation using the Venant method. (for fast debugging/testing, you can use the 3 layers head model with a coarse mesh and a basic source space, with less than 20 electrodes, here is my testing model 👍
)

So, regarding my last modifications, the duneuro binaries will not be copied to the bst temporary folder, but before running the binaries we 'cd' to the \bin\ all the related files are created on the \tmp\ folder, then used by Duneuro from the \bin\ folder

However, the outputs of the binaries are in the \bin\ folder (the leadfiled matrix and the transfer matrix) So, for now, bst will read the G matrix from this folder. You can find the "TODO" word, it related to the parts where I was either stuck or it needs further coding/optimisation.

Some explanations about the binaries: there are mainly two outputs 1- the leadfield (LF) matrix or the Gain matrix (G) 2- the transfer matrix

The LF is used immediately, and the TM is just saved for now. WHY? The transfer matrix (TM) is important, 80% (or more) of the FEM computation time is related to this matrix. To compute the LF, only a simple multiplication of the TM by the FEM source space is performed. So, in the case where the user changes ONLY the source space, the LF computation will be much faster. However, if ONE of these parameters changes:

• Head model (mesh, nb of tissue ...)
• Conductivity value
• Electrodes' positions

The TF needs to be computed again,

So, in the scenario, where the user changes the cortex (source space) and want to compute again the FEM, we can call this matrix.

The duneuro binaries are already programmed to check if there is any TM in the current folder. We need now to manage all this from the bst side.

[ftadel]

However, the outputs of the binaries are in the \bin\ folder (the leadfiled matrix and the transfer matrix). So, for now, bst will read the G matrix from this folder.

They don't have any way of specifying the output folder? It seems very unlikely that they designed their program so that it only works with the output data in the binary folder: binaries are most of the time supposed to be in folders where the user doesn't have the write rights... If there is no such feature, it should be reported to them and fixed.

So, in the scenario, where the user changes the cortex (source space) and want to compute again the FEM, we can call this matrix.

We had coded optimized things like this for OpenMEEG as well, to reuse intermediate computation steps, but ended up dropping them. It was causing more confusion than it was helping, and it was making the updates of the software more complicated... In practice, most Brainstorm users do not compute different forward models, especially not with different source spaces. If there is something that they compute multiple times, it's the inverse solutions. If they compute different forward models, it is to compare different methods (ie. same source space, but to compare a spherical model with a BEM).

I think storing this transfer matrix would be useful almost only for the developers (you, Juan, myself). For other users and a standard processing pipeline, it can bring more trouble than advantages. Maybe we can find a slightly hacked way of reusing this matrix (for instance, by not emptying the temporary folder at the beginning of the process, so that duneuro reuses the existing files), but only for advanced user as a debugging feature, and documenting it in an advanced section of the tutorial. It could be done by setting a global variable for instance.

Maybe it's not necessary to spend time on making this a public standard feature at the moment.

[juangpc]

Hi, This is so cool!! what you guys are doing with the meshes. It is great. 🥇

Regarding the discussion of saving the transfer matrix. I'm sorry to disagree but I do think that it makes a lot of sense to save it. For instance, one of the scenarios where this all might become truly useful is in seeg, where you might want to check different electrode positions before surgery. I think that, since it is already working, I would leave it. Besides, nobody "really needs" to understand the dynamics of it all. Some users will notice the second time you compute a similar model the computation is much faster, but they don't really need to know. Still, let me offer a few ideas:

• I could try to make make the matrix persistent. There are, as far as I've been reading, several solutions libraries in c++ that help you achieve this. This way it will be, implicitly persistent for matlab. As far as any call to the function knows... it will be there. The only problem is that this will be at the cost of main memory of the users computer. If the user doesn't have too much memory... this can all turn quite unstable.
• I can add another input to the cpp code so that we have an output folder specified in the call.

They don't have any way of specifying the output folder? It seems very unlikely that they designed their program so that it only works with the output data in the binary folder: binaries are most of the time supposed to be in folders where the user doesn't have the write rights... If there is no such feature, it should be reported to them and fixed

by "they" and "them" you are actually referring to probably @tmedani and myself 😸 😸

[ftadel]

How big is this transfer matrix? With OpenMEEG, the temporary files we were saving were several hundreds of Mb, that were almost never used. That was another reason not to keep it. The databases are big enough, if we can save a few Gb here and there, it's always interesting. If it's big, maybe we can make it an option to save it (since we'll already need an option panel, it wouldn't cost much to have an extra checkbox).

I could try to make make the matrix persistent

No, let's avoid packing the memory...

I can add another input to the cpp code so that we have an output folder specified in the call

It would be good to have a solution with which 1) we don't have to copy the binaries 2) we don't need to save the output in the binary directory (in the compiled version, the binary folder will be embedded in the brainstorm distribution and possibly read-only). So yes, an output folder option would be good.

by "they" and "them" you are actually referring to probably @tmedani and myself

Oops, I didn't realize that this step was coded by you guys, I thought it was coming from the duneuro people :)

[tmedani]

How big is this transfer matrix?

Not too big, its size is [Number of Sensor X Number of Source]

From this answer, I think I wrote something wrong in my previous post, sorry guys!. The transfer matrix depends on the FEM source model and not the source space (also source model but does not mean the same thing!). It's confusing me, and for sure I confused you... Sorry! I need to review the theory and I will come back to you with a more accurate answer. In any way, the transfer matrix is useful to have it.

1) we don't have to copy the binaries

This is already solved in the last version of bst-duneuro code.

2) we don't need to save the output in the binary directory

We will updates the binaries asap and add an output directory. @juangpc we need to discuss this, I think the best way to do it is to add a variable on the INI file and then read it.

[juangpc]

Hi, I've been doing some testing the cpp app seems to work.

Nice input Tak. Yes I now have a way to specify the output folder within the .ini file And it works. I still need to push a few details but it seems to be working. I'll push it sometime next week.

But I guess, now my question is. How do you want to use it within brainstorm? Or in other words... we can:

• We can ask brainstorm to modify the *.ini file with the location of the temp folder for every installation of brainstorm. and then call the bst-duneuro app which will read this address from the text file.
• We can include a separate parameter like --output /home/username which would be easier for brainstorm because we do know where the temp folder will be. I think this might be less elegant but a bit more convenient since we are doing a system call from brainstorm, which implies creating a string for the system call.

By the way, both relative and absolute paths will work in both options. let me know which one you prefer and I'll compile and push.

[tmedani]

Hi @juangpc , This is super! bravo.

For your question

How do you want to use it within brainstorm?

As it is implemented now, when it's needed, bst-duneuro toolbox will be download and installed on .brainstorm\ and when the FEM is called, all the temporary folders (dipole, electrode, mesh ....) will be saved to .brainstrom\tmp for now, only the binaries outputs are on bst-duneuro\bin but with your modification, they will be also moved to the tmp folder, which will keep the bst-duneuro/bin clean so, we need just to add on the ini file the value of the outputfolder like this : outputfolder = .brainstorm\tmp Then we are done with this :) @ftadel do you agree?

[juangpc]

ok thanks

how do we transform .brainstorm/tmp into something the cpp app will understand?

the app only knows either absolute paths or relative to from where the app is running from, which in our case will be from where matlab is calling it from. and that info needs to be inside the text ini file and correct for every brainstorm/matlab installation.

On Fri, Jan 31, 2020 at 5:43 PM Takfarinas MEDANI notifications@github.com wrote:

Hi @juangpc https://github.com/juangpc , This is super! bravo.

For your question

How do you want to use it within brainstorm?

As it is implemented now, when it's needed, bst-duneuro toolbox will be download and installed on .brainstorm\ and when the FEM is called, all the temporary folders (dipole, electrode, mesh ....) will be saved to .brainstrom\tmp for now, only the binaries outputs are on bst-duneuro\bin but with your modification, they will be also moved to the tmp folder, which will keep the bst-duneuro/bin clean so, we need just to add on the ini file the value of the outputfolder like this : outputfolder = .brainstorm\tmp Then we are done with this :) @ftadel https://github.com/ftadel do you agree?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/brainstorm-tools/bst-duneuro/issues/1?email_source=notifications&email_token=ACEKMIAKWDSNEFEBF7ETRHDRASZTNA5CNFSM4J4YVMMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKQLQMI#issuecomment-580958257, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACEKMIAQHGNHKTCD4DY2QK3RASZTNANCNFSM4J4YVMMA .

-- Juan

[juangpc]

Or maybe from were th binary file is? need to check it and test it. but this is why the input option seems a bit less complicated.

By the way... if the user wants to modify other options in the ini file, how will bst modify it?

On Fri, Jan 31, 2020 at 5:53 PM Juan juangpc@gmail.com wrote:

ok thanks

how do we transform .brainstorm/tmp into something the cpp app will understand?

the app only knows either absolute paths or relative to from where the app is running from, which in our case will be from where matlab is calling it from. and that info needs to be inside the text ini file and correct for every brainstorm/matlab installation.

On Fri, Jan 31, 2020 at 5:43 PM Takfarinas MEDANI < notifications@github.com> wrote:

Hi @juangpc https://github.com/juangpc , This is super! bravo.

For your question

How do you want to use it within brainstorm?

As it is implemented now, when it's needed, bst-duneuro toolbox will be download and installed on .brainstorm\ and when the FEM is called, all the temporary folders (dipole, electrode, mesh ....) will be saved to .brainstrom\tmp for now, only the binaries outputs are on bst-duneuro\bin but with your modification, they will be also moved to the tmp folder, which will keep the bst-duneuro/bin clean so, we need just to add on the ini file the value of the outputfolder like this : outputfolder = .brainstorm\tmp Then we are done with this :) @ftadel https://github.com/ftadel do you agree?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/brainstorm-tools/bst-duneuro/issues/1?email_source=notifications&email_token=ACEKMIAKWDSNEFEBF7ETRHDRASZTNA5CNFSM4J4YVMMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKQLQMI#issuecomment-580958257, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACEKMIAQHGNHKTCD4DY2QK3RASZTNANCNFSM4J4YVMMA .

-- Juan

-- Juan

[tmedani]

how do we transform .brainstorm/tmp into something the cpp app will understand?

This is already the case for the temporary file, here is an example : https://github.com/brainstorm-tools/bst-duneuro/blob/7b7d0bb260b691ab6298ac3927ff64dbc6b7e802/matlab/write_duneuro_minifile2.m#L49

Since the app can read the input files, I think it could also write.

and that info needs to be inside the text ini file and correct for every brainstorm/matlab installation.

Yes, it will work on any machine, it's not an absolute path, but related to the installation/pc. bst can manage this.

By the way... if the user wants to modify other options in the ini file, how will bst modify it?

Good question ... this function "write_duneuro_minifile2.m" write all the input information to the mini/ini file. All these parameters are stored on the cfg structure.

I summarized most of these parameters here: https://docs.google.com/spreadsheets/d/1MqURQsszn8Qj3-XRX_Z8qFFnz6Yl2-uYALkV-8pJVaM/edit#gid=0

We need to build a big panel, from bst that will set the most important parameters. For now, I set most of them to the default value, but later, we will need this panel (similar to the one used by the inverse problem)

Since there are also different source models for the forward solution, we can build the same logic as the inverse, since with the FEM, we can have also different forward solutions.

[ftadel]

outputfolder = .brainstorm\tmp

The temporary folder is not necessarily inside the $HOME/.brainstorm folder. One typical use case on Linux is to have a limited amount of space available in the $HOME folder. It is possible to change the temporary folder from the brainstorm preferences. Therefore you can't guess where the temp folder would be, it has to be an input parameter easy to edit. So probably better if there is a command option for that.

[ftadel]

We need to build a big panel, from bst that will set the most important parameters. For now, I set most of them to the default value, but later, we will need this panel (similar to the one used by the inverse problem)

I can start working on it this week if you want. Do you already have some options you'd like to change? If so, please list the names, types and possible values for each option.

[tmedani]

I can start working on it this week if you want. Do you already have some options you'd like to change? If so, please list the names, types and possible values for each option.

Hi @ftadel, I will work on that asap, maybe not this week. I would like to finish the validation of the MEG computation within brainstorm first.

[juangpc]

Hi, Just pushed the working binaries.

• The rebuilding tools are working just fine. With a single command (setup_bst_duneuro.sh rebuild all duneuro-matlab I get both win and linux ready, only rebuilding the minimally needed. At some point, I'll try to add mac to the tool.
• @ftadel, the option to include an input folder spec in the command line worked fine. I did some testing yesterday and if you want I can implement it back. But, I thought that rather than burning into the code the name of the transfer and lead field files, it would be better to have it set as a parameter, so that added up into too many input parameters for a reasonable command-line call. So I decided to check and implement the option to read it from the mini file. There is a whole set of tools in duneuro for this purpose so it really makes sense to use them. Right now, in the mini file, I've created a new subset of input parameters. You interact with this info in the source like this...

  config["brainstorm.workingfolder"] + config["brainstorm.meg_leadfield_filename"]

So, as you can see it ends up being quite handy. Now, from brainstorm, we can interchange outputfolder --> which I call workingfolder and other names easily.

As I said, I can reimplement back the command line output folder set easily. Your call!. By the way, right now, if the output folder is not found, the output will get lost and not saved. So matlab or whoever should have the responsibility of checking whether the output folder exists indeed.

• @tmedani I have doubts on how you've implemented the meeg option. If the user wants meeg, why not calling first eeg and then meg instead of a separate meeg? It breaks a little bit the code flow right now.

[tmedani]

Hi @juangpc , Great for the updates on the CPP, thanks.

@ftadel this may involve some changes on the way of brainstorm call duneuro, It needs to be updated according to the new version!

Have you started reviewing the code from your side?

• @tmedani I have doubts on how you've implemented the meeg option. If the user wants meeg, why not calling first eeg and then meg instead of a separate meeg? It breaks a little bit the code flow right now.

It's much faster to read all the input data (mesh, tensors, sensors, and sources) and then construct the duneuro driver on one-shot both for combined EEG and MEG. Dividing the process into EEG then in MEG will slow more the process.

What was wrong with the MEEG option?

[tmedani]

Hello, Regarding our last discussion,

How big is this transfer matrix?

The right answer is : [Number of Node X Number of Sensor]

So, my first explanation was correct, https://github.com/brainstorm-tools/bst-duneuro/issues/1#issuecomment-580563742

My error was in the second response where my answer confused my thoughts. My previous answer refers to the size of the LF matrix with a constrained orientation and not to the TM.

How big is this transfer matrix?

Not too big, its size is [Number of Sensor X Number of Source]

From this answer, I think I wrote something wrong in my previous post, sorry guys!. The transfer matrix depends on the FEM source model and not the source space (also source model but does not mean the same thing!). It's confusing me, and for sure I confused you... Sorry! I need to review the theory and I will come back to you with a more accurate answer. In any way, the transfer matrix is useful to have it.

So, TM depends only on the head model (mesh and conductivity) and the electrodes' position ( and also a set of hidden parameters related to the FEM method and the solvers).

To compute the lead field (LF) matrix, simple multiplication is performed between the TM and the FEM Source Model, which is much faster than to recompute the whole forward problem.

@ftadel, for the advanced user, or for research purpose, Duneuro offers a panel of the source models (Venant, Subtraction, PI ....) with subparameters, Therefore, keeping TM is important to compare the performance of the different FEM source models. And probably, soon, we will have students (from Germany) that we will perform comparisons on FEM source model using brainstorm, so I think it's important to have this TM reusable.

[ftadel]

@juangpc

it would be better to have it set as a parameter, so that added up into too many input parameters for a reasonable command-line call

What's a reasonable command-line call? We don't care much about how many inputs parameters we have if this is all written automatically. OpenMEEG calls can be quite invasive as well.

Right now, in the mini file, I've created a new subset of input parameters.

If is OK to pass parameters through a .ini file, as long as this ini file is always with write access. It can't be located in the bin folder (which might be read-only), Brainstorm should be able to write this file in its user-defined temporary folder (bst_get('BrainstormTmpDir')).

config["brainstorm.workingfolder"] + config["brainstorm.meg_leadfield_filename"]

This won't be available from Matlab/Brainstorm. Passing input parameters from Brainstorm to duneuro means that we need to find or develop functions to read/write these .ini files. Libraries already seem to exist (eg. https://www.mathworks.com/matlabcentral/fileexchange/24992-ini-config), but this will require some additional integration, and redistribution of new files.

If you think this is useful for more future flexibility, let's do it. If it's just for passing the path to the temporary folder and some extra file names and you already have code working for passing them through the command line: let's use the command line.

As I said, I can reimplement back the command line output folder set easily. Your call!. By the way, right now, if the output folder is not found, the output will get lost and not saved. So matlab or whoever should have the responsibility of checking whether the output folder exists indeed.

Is it complicated to add an error message saying that the folder doesn't exist or is read-only?

A generic comment about handling files: It is difficult to check that the user has really the rights to write in the output folder. This case should be handled carefully because it's a common problem but sometimes difficult to debug, and we should be able to report an explicit message instead of an ugly crash or and absence of result.

@tmedani

Have you started reviewing the code from your side?

Not yet, I'm a bit confused with what I'm supposed to do at the moment, because there seem to be lots of changes in bst-duneuro, and nothing seems to follow on the brainstorm side. Do you think I should start working on making the integration smoother and cleaner now in parallel with your methodological developments, or do you think it would be confusing for you and I should wait until you're done with all your current developments? There is no rush, I have plenty of other things to work on... :)

for the advanced user, or for research purpose, Duneuro offers a panel of the source models (Venant, Subtraction, PI ....) with subparameters, Therefore, keeping TM is important to compare the performance of the different FEM source models. And probably, soon, we will have students (from Germany) that we will perform comparisons on FEM source model using brainstorm, so I think it's important to have this TM reusable.

OK, I got it, you both want to keep it, we'll keep it :-) I would still like to avoid storing this file in the database permanently for users who don't know what they're doing. Could we work this out in either of these ways: 1) We save it in the database folders ONLY if an explicit option "Save transfer matrix" is selected (disabled by default). Then we need to find a reliable way to re-use it: where do you want to save this in the database? and how are you planning to make sure you are reusing the correct existing file? how do you make sure the input files were not changed by the user between two calls to bst-duneuro? etc... 2) We do not save it in the database, but leave it in the temporary folder (with no additional input option needed). When calling bst-duneuro, we empty the temporary folder but keep all the partial results that could be reused (in case the input meshes or parameters did not change). As part of Brainstorm temp folder, it is wiped-out completely when Brainstorm is restarted. This makes it easier to check if the inputs are correct, without storing too much extra stuff in the database. The successive computations would be faster (and benchmarkable) but only for successive calls, not permanently. I like this solution a lot better. Could this be satisfying enough or you?

[tmedani]

Hello, So regarding the ini file,

If is OK to pass parameters through a .ini file, as long as this ini file is always with write access. It can't be located in the bin folder (which might be read-only), Brainstorm should be able to write this file in its user-defined temporary folder (bst_get('BrainstormTmpDir')).

in bst-dueneuro I wrote some functions dedicated to write the ini for duneuro.

https://github.com/brainstorm-tools/bst-duneuro/blob/master/matlab/write_duneuro_minifile2.m

These functions can convert all the input from brainstorm to the ini file and then can be saved in any place.

So, with the new modification on the binary, no data will be saved on the bin. all the intermediate files will be saved on the tmp folder and we can specify the destination of the output.

I will apply the change according to the new version of the binary asap @jaun will push the change.

Is it complicated to add an error message saying that the folder doesn't exist or is read-only?

A generic comment about handling files: It is difficult to check that the user has really the rights to write in the output folder. This case should be handled carefully because it's a common problem but sometimes difficult to debug, and we should be able to report an explicit message instead of an ugly crash or and absence of result.

I think DUNE handles all these problems. The error messages returned are explicit. I have already added a log file related to the output of the binary in the case of errors and will be saved as well on the temporary folder : a

https://github.com/brainstorm-tools/bst-duneuro/blob/bf44757a9d308d6f40d2f3fd9097a09c279f3f07/matlab/bst_run_duneuro_cmd.m#L43

it will save as well the cfg structure used by duneuro for advanced checking on the data.

Have you started reviewing the code from your side?

Not yet, I'm a bit confused with what I'm supposed to do at the moment, because there seem to be lots of changes in bst-duneuro, and nothing seems to follow on the brainstorm side. Do you think I should start working on making the integration smoother and cleaner now in parallel with your methodological developments, or do you think it would be confusing for you and I should wait until you're done with all your current developments? There is no rush, I have plenty of other things to work on... :)

As it is now, the Duneuro head model for EEG works fine even with these modifications. However, you can wait until these last changes.

OK, I got it, you both want to keep it, we'll keep it :-)

Thanks :)

I would still like to avoid storing this file in the database permanently for users who don't know what they're doing. Could we work this out in either of these ways:

Good idea, I think we can add this option on the BIG panel for the Duneuro parameters. I will think of the best way how to manage this matrix (unique tag, place, ...) we can put this as an optimization part. let keep it in the output folder for now, and we will see where it should be.

@juan could you add a boolean flag to the ini file in order to save or not the transfer matrix.

[juangpc]

Hi both, I appreciate your attention to this Francois. I just want to make it work which ever way you guys think is most desirable.

About the call. True. Even simnibs' call to matlab is like a big paragraph. But it is also true that we are already using ini file to set a whole bunch of parameters, so why not use it. I have implemented this option following Taks ideas.

I've just pushed the following features:

• application saves to a specific route. Still it doesn't check for either existence or writing permission. I need a bit more time. So no more saving into /bin folder nor copying between /bin and /temp, etc...
• all input parameters are done through ini file.
• following Taks comments, the transfer file is only saved if the user "knows what he/she is doing" and modifies this flag in the ini file. I think a bit more organized.
• some more input parameters, like modality (eeg,meg or meeg), or save_eeg_transfer_file which has to be "true" in order to save the file (it doesn't really check it to be false... it just checks wether it is equal to "true" or not). Note. I am using snake case here. If you prefer camel case, let me know. Right now, ini files use both so there is a bit of homogeneity there. Which do you prefer? [image: image.png]

Juan

On Wed, Feb 5, 2020 at 1:10 PM Takfarinas MEDANI notifications@github.com wrote:

Hello, So regarding the ini file,

If is OK to pass parameters through a .ini file, as long as this ini file is always with write access. It can't be located in the bin folder (which might be read-only), Brainstorm should be able to write this file in its user-defined temporary folder (bst_get('BrainstormTmpDir')).

in bst-dueneuro I wrote some functions dedicated to write the ini for duneuro. These functions can convert all the input from brainstorm to the ini file and then can be saved in any place.

So, with the new modification on the binary, no data will be saved on the bin. all the intermediate files will be saved on the tmp folder and we can specify the destination of the output.

I will apply the change according to the new version of the binary asap @jaun https://github.com/jaun will push the change.

Is it complicated to add an error message saying that the folder doesn't exist or is read-only?

A generic comment about handling files: It is difficult to check that the user has really the rights to write in the output folder. This case should be handled carefully because it's a common problem but sometimes difficult to debug, and we should be able to report an explicit message instead of an ugly crash or and absence of result.

I think DUNE handles all these problems. The error messages returned are explicit. I have already added a log file related to the output of the binary in the case of errors and will be saved as well on the temporary folder.

Have you started reviewing the code from your side?

Not yet, I'm a bit confused with what I'm supposed to do at the moment, because there seem to be lots of changes in bst-duneuro, and nothing seems to follow on the brainstorm side. Do you think I should start working on making the integration smoother and cleaner now in parallel with your methodological developments, or do you think it would be confusing for you and I should wait until you're done with all your current developments? There is no rush, I have plenty of other things to work on... :)

As it is now, the Duneuro head model for EEG works fine even with these modifications. However, you can wait until these last changes.

OK, I got it, you both want to keep it, we'll keep it :-)

Thanks :)

I would still like to avoid storing this file in the database permanently for users who don't know what they're doing. Could we work this out in either of these ways:

Good idea, I think we can add this option on the BIG panel for the Duneuro parameters. I will think of the best way how to manage this matrix (unique tag, place, ...) we can put this as an optimization part. let keep it in the output folder for now, and we will see where it should be.

@juan https://github.com/juan could you add a boolean flag to the ini file in order to save or not the transfer matrix.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/brainstorm-tools/bst-duneuro/issues/1?email_source=notifications&email_token=ACEKMIEKUTTEQDTYRM3TGDTRBMFLRA5CNFSM4J4YVMMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK4TRCA#issuecomment-582563976, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACEKMIGGTE52BCURC6TI3ITRBMFLRANCNFSM4J4YVMMA .

[tmedani]

Hey La Team,

@juangpc thanks for the last updates, this is great.

I have just pushed the last updates that take into account the new modification on the Duneuro applications.

There is a slight error in the concatenation of the outputfolder + nameOfFile (there is no separator) I solved this from the ini file, so it should be fine for now.

https://github.com/brainstorm-tools/bst-duneuro/blob/d5573e8eafeb3f5f3fd039c5fdf2f39b682c65ab/matlab/bst_write_duneuro_minifile.m#L145

Now, all the process is clean, no copying, no output on the bin folder...

All the temporary files will be saved into ./brainstorm/tmp

The outputs folder is specified on the ini file and for now I put it to the ./brainstorm/tmp as default value.

However, I found an exception, in my computer, I have the bst-duneuro\bin in a folder with a space in the path 'G:\My Drive\bst_integration\duneuro_interface\bst-duneuro' in that case, the system call to the binaries fails. So, in this situation, the temporary solution that I implemented is to move the binaries to the tmp folder and run all from this folder. (this should not happen for regular users, but just in case)

@ftadel in the case where the user changes the location of the tmp folder from the preference, ./brainstorm/tmp
since we can get it with : bst_get('BrainstormTmpDir') so there is no problem even in this case.

Also, you can start reviewing the code.

maybe this image can help for an overview (progress color is outpdated)

application saves to a specific route. Still it doesn't check for either existence or writing permission

I have just added a check for this in the begining of the duneuro process : https://github.com/brainstorm-tools/bst-duneuro/blob/873e679decdb54002788e181f7c044c37edc02ad/matlab/bst_duneuro_interface.m#L54

[ftadel]

in that case, the system call to the binaries fails. So, in this situation, the temporary solution that I implemented is to move the binaries to the tmp folder and run all from this folder. (this should not happen for regular users, but just in case)

Let's consider this a regular case. You have no idea how messy people can be, spaces and weird characters in folder names are common. You should find a robust way to make this work. Typically all file and folder names should be passed with double quotes around them (eg. "C:\Program Files\brainstorm3...") It's sometimes a bit of a headache to escape correctly the strings when they are evaluated successively by different programs, but it's most of the time doable.

Also, you can start reviewing the code.

I started working on some other things, will get back to this next week.

[juangpc]

Hi, doing some testing. there is a class called "parametertreeparser" and it should handle correctly either single or double quotes. So you can use the if you want. However the parser is working fine, I think the problem is actually later in the code, when opening some of the filews, not actually in the parser. So, @tmedani could you try to add double \ instead of a single one. This might help the os identify when you have scape characters and when you dont, and then it will correctly parse a space.

[juangpc]

There is a slight error in the concatenation of the outputfolder + nameOfFile (there is no separator) I solved this from the ini file, so it should be fine for now.

@tmedani sorry I'm not following. What is the error?

[tmedani]

There is a slight error in the concatenation of the outputfolder + nameOfFile (there is no separator) I solved this from the ini file, so it should be fine for now.

@tmedani sorry I'm not following. What is the error?

The name of the output files from the binaries, a separator is missing during the concatenation between the outputfolder and the name of the file example: outputfile = C:\Users\33649.brainstorm\tmpeeg_lf.dat instead of : outputfile = C:\Users\33649.brainstorm\tmp\eeg_lf.dat

[tmedani]

So, @tmedani could you try to add double \ instead of a single one. This might help the os identify when you have scape characters and when you dont, and then it will correctly parse a space.

my turn : I did not understand how to do it? could we chat on skype?

[juangpc]

Hi @tmedani, sorry I wasn't understanding you. After several tests I can confirm that if you put a path in the ini file either as a text, with or without spaces, or using single or double quotes, it will be parsed correctly and used.

But as I understand it right now, the problem is in fact inside matlab, not the cpp. The way matlab handles the spaces... I'll show you the fix so you can solve the issue yourself, I don't want to mess around with the scripts while you're working with them. I've done some testing and the call is working just fine right now. You just need to suround your path to the binary in double-quotes. I've created to separate folders in my pc: my folder and my other folder. If copy the binary into the 1st folder, an ini file into the 2nd, and you run the following code...

clear
bstfpath = 'C:\projects\my folder';
bstfname = 'bst_duneuro_meeg';
bstext = '.exe';
inifpath = 'C:\projects\my other folder';
inifname = 'eeg_transfer_cg_0001.ini';
arghelp = '--h';

callStr1 = [ bstfpath  filesep bstfname bstext ' ' inifpath filesep inifname ' ' arghelp];
callStr2 = [ '"' bstfpath  filesep bstfname bstext '" "' inifpath filesep inifname ' ' arghelp '"'];
callStr3 = [ '"' bstfpath  filesep bstfname bstext '" ' inifpath filesep inifname ' ' arghelp];
callStr4 = [ '"' bstfpath  filesep bstfname bstext ' ' inifpath filesep inifname ' ' arghelp '"'];

system(callStr1)
system(callStr2);
system(callStr3);
system(callStr4);

>> system(callStr1) 'C:\projects\my' is not recognized as an internal or external command, operable program or batch file. ans = 1

>> system(callStr2); Dune reported error: Dune::IOError [readINITree:/home/juan/bst-duneuro/src/dune-common/dune/common/parametertreeparser.cc:49]: Could not open configuration file C:\projects\my other folder\eeg_transfer_cg_0001.ini --h

>> system(callStr3);

BST_DUNEURO_MEEG 
This script computes lead fields for EEG, MEG and combined MEG/EEG forward problem. 
Based on duneuro softweare (www.duneuro.org) 
Designed to work with Brainstorm Toolbox (https://neuroimage.usc.edu/brainstorm)       
Created initially by the Duneuro team. 
Adapted and modified by Takfarinas Medani and Juan Garcia-Prieto 

Usage: bst_duneuro_meeg config_file.ini --mode 

    - config_file.ini 
      Configuration file containing the parmeters of the forward model to compute. 

    - mode: {eeg, meg, meeg} 

This application computes the MEG, MEG and combined MEG/EEG transfer matrix and the final leadfields solution 
If the source/sensor models are not modified, the application will search for a previously computed transfer 
matrix (stored in eeg_transfer.dat or meg_transfer.dat binary files. 
The final output leadfiedl martix will be stored in a binary and a text file named eeg_lf.dat/txt and/or 
meg_lf.dat/txt 

Examples: 

    - bst_duneuro --help                                 Will print this help. 

    - bst_duneuro eeg_config_file.mini --eeg             Will compute solution for eeg. 

    - bst_duneuro meg_config_file.mini --meg             Will compute solution for meg. 

    - bst_duneuro meeg_config_file.mini --meeg           Will compute solution for both/combined meg and eeg. 

>> system(callStr4); The filename, directory name, or volume label syntax is incorrect.

-- Short answer... none of the above really work. It is interesting. What really happens is that you have to include open and close double quote markes in every argument of a system call. First argument will be the actual binary path and filename with extension. Following arguments would be... the ini file or the '--h' if you want help.

so...

callStr5 = [ '"' bstfpath  filesep bstfname bstext '"' ' ' '"' inifpath filesep inifname '"' ' ' '"' arghelp '"'];
callStr6 = [ '"' bstfpath  filesep bstfname bstext '"' ' ' '"' inifpath filesep inifname '"'];
system(callStr5);
system(callStr6);

>> system(callStr5);

BST_DUNEURO_MEEG 
This script computes lead fields for EEG, MEG and combined MEG/EEG forward problem. 
Based on duneuro softweare (www.duneuro.org) 
Designed to work with Brainstorm Toolbox (https://neuroimage.usc.edu/brainstorm)       
Created initially by the Duneuro team. 
Adapted and modified by Takfarinas Medani and Juan Garcia-Prieto 

Usage: bst_duneuro_meeg config_file.ini --mode 

    - config_file.ini 
      Configuration file containing the parmeters of the forward model to compute. 

    - mode: {eeg, meg, meeg} 

This application computes the MEG, MEG and combined MEG/EEG transfer matrix and the final leadfields solution 
If the source/sensor models are not modified, the application will search for a previously computed transfer 
matrix (stored in eeg_transfer.dat or meg_transfer.dat binary files. 
The final output leadfiedl martix will be stored in a binary and a text file named eeg_lf.dat/txt and/or 
meg_lf.dat/txt 

Examples: 

    - bst_duneuro --help                                 Will print this help. 

    - bst_duneuro eeg_config_file.mini --eeg             Will compute solution for eeg. 

    - bst_duneuro meg_config_file.mini --meg             Will compute solution for meg. 

    - bst_duneuro meeg_config_file.mini --meeg           Will compute solution for both/combined meg and eeg. 

>> system(callStr6); Dune reported error: DGFException [DGFGridFactory:/home/juan/bst-duneuro/src/dune-grid/dune/grid/io/file/dgfparser/dgfug.hh:107]: Error: Macrofile test_sphere_hex.dgf not found

Both callStr5 or callStr6 will work JUST FINE! so @tmedani what needs to be done is to generate a call concatenating with spaces diferent arguments. Each argument with double quotes... that way it will work!

[juangpc]

By the way, we need to update the help message!

[tmedani]

so @tmedani what needs to be done is to generate a call concatenating with spaces different arguments. Each argument with double quotes... that way it will work!

Super Juan .... thanks for the super-review. I have just updated the function call.

By the way, we need to update the help message!

indeed... still a lot to do...