brainstorm-tools / brainstorm3

Brainstorm software: MEG, EEG, fNIRS, ECoG, sEEG and electrophysiology
http://neuroimage.usc.edu/brainstorm
GNU General Public License v3.0
385 stars 162 forks source link

FEM tutorial: Issues to fix #431

Open ftadel opened 3 years ago

ftadel commented 3 years ago

1) SimNIBS: Remeshing the SimNIBS model with iso2mesh doesn't work. Here is the error message:

    FEM> 1. scalp: C:\Work\Protocols\TutorialFem\anat\Subject01\tess_scalp.mat
    FEM> 2. skull: C:\Work\Protocols\TutorialFem\anat\Subject01\tess_skull.mat
    FEM> 3. white: C:\Work\Protocols\TutorialFem\anat\Subject01\tess_white.mat
    FEM> 4.  gray: C:\Work\Protocols\TutorialFem\anat\Subject01\tess_gray.mat
    FEM> 5.   csf: C:\Work\Protocols\TutorialFem\anat\Subject01\tess_csf.mat

    generating tetrahedral mesh from closed surfaces ...
    creating volumetric mesh from a surface mesh ...

    ***************************************************************************
    ** Error: Line 117:  surf2mesh (line 117)
    ** Tetgen command failed
    **
    ** Call stack:
    ** >surf2mesh.m at 117
    ** >process_fem_mesh.m>Compute at 442

2) Pre-processing: The PSD screen capture does not seem to be from this file (not the same time definition) 3) Averaging: The averaging screen capture shows a weighted average: this is confusing => Uncheck the "weighted" box 4) Noise covariance:

ftadel commented 3 years ago

@juangpc @tmedani

I also posted a draft for the processing script: https://neuroimage.usc.edu/brainstorm/Tutorials/FemMedianNerve#Scripting https://github.com/brainstorm-tools/brainstorm3/blob/master/toolbox/script/tutorial_fem.m

Please use try to use it to reproduce your computations: it will allow to automate the rebuild of everything when we change a parameter in the script or when we update Brainstorm or Duneuro.

For addressing the issues: please post your comments as you work on them, and I will edit the first comment of this thread to keep track of what is left in the todo list.

Thanks!

juangpc commented 3 years ago

Will do.

As a start. I can't unzip iso2mesh's zip folder when installing the plugin. I get "An internal error has occurred." I'm trying to see what's the matter. Not related to permissions, nor incorrect paths. Maybe zip versions? I'm running Matlab 2021a, Win 10 x64 . The zip issue, seems to be something not working correctly within matlab. The folder is unzipped, but somehow there is an error thrown. I can zip the files myself and then unzip works fine. It works through querying matlab's to see if 'ZIPV2' feature is installed. No more java based unzipping. If this continues like this, suggest try/catch wrapping the unzipping here.

ftadel commented 3 years ago

I'm also running Matlab 2021a/Win10 and I have no problem with this file: https://github.com/fangq/iso2mesh/releases/download/v1.9.2/iso2mesh-1.9.2-allinone.zip So I guess this issue is specific to your installation of Matlab / your system.

Can you please try some further debugging? 1) Open unzip.m in Matlab, put a break point at line 65, then execute step by step until you find the offensive line. 2) What does feature('ZIPV2')==1 give you? 3) Do you get this with any other zip file? What about other .zip files downloaded from Github? (like Brain2mesh). 4) Do you have other Matlab versions installed that you can test? Alternatively, try this attached function (from Matlab 2020a): unzip_2020a.zip

If this is only one specific file that can't be unzipped with your Matlab and the 2020a version works : Could you please open a bug report on the Mathworks website? This "ZIP2" is a new feature added in 2021a (or 2020b), with the objective of removing Java from Matlab. They need to fix it ASAP if it's not working correctly... Send them the .zip that doesn't work, and they will probably provide you with a workaround that we'll be able to use.

juangpc commented 3 years ago

I do have a problem with that zip file. But it really looks like it is a Matlab problem, rather than a BST one.

What does feature('ZIPV2')==1 give you?

feature('ZIP2')==1 returns True.

Open unzip.m in Matlab, put a break point at line 65, then execute step by step until you find the offensive line.

Line 103 from unzip.m throws specifically when unzipping this file. That is catched and the output is: "An internal error has occurred". The stack provided by the exception includes only a call to unzip itself (line 103). The error can be reproduced calling unzip with different or no varargins.

Do you have other Matlab versions installed that you can test?

No other versions of Matlab installed.

Do you get this with any other zip file? What about other .zip files downloaded from GitHub? (like Brain2mesh).

I've tried unzipping independently of BST and it still fails. Also tried in the Desktop folder to avoid problems with permissions and it still fails.

I've created a MathWorks bug report (Case Number: 05023039, Subject: ZIPV2 bug). The bug report system doesn't allow to send a 25mb file, but I sent them the link from github(https://github.com/fangq/iso2mesh/releases/download/v1.9.2/iso2mesh-1.9.2-allinone.zip). (Perhaps this can be forwarded if you know someone working specifically on this within MathWorks.

Let's see.

ftadel commented 3 years ago

I've created a MathWorks bug report

Thanks! Let's see what they recommend. Please forward the response when you get one.

juangpc commented 3 years ago

SimNIBS: Remeshing the SimNIBS model with iso2mesh doesn't work. Here is the error message:

Run through the simnibs part myself and everything seems to be working.

juangpc commented 3 years ago

Once the SimNIBS is working (think it is).

Fiducials:

This subject has his preauricular fiducials located in a somewhat particular location. Apart from the MRI themselves, we also have these captures of the fiducial points locations. Should we include these captures?

image image image

With this information my results locate the fiducials at these locations:

| |- NAS: [120.9375, 218.4375, 44.9995] | |- LPA: [43.125, 109.6875, 25.9997] | |- RPA: [200.625, 110.625, 29.9997] | |- R: [-0.0085529, 0.98786, 0.15509; -0.99967, -0.0047002, -0.025192; -0.024157, -0.15526, 0.98758] | |- T: [-112.1194; 123.0581; -7.6054] | |- Origin: [121.875; 110.1562; 27.9997] |- NCS:
| |- AC: [120.9375, 126.5625, 73.9992] | |- PC: [120.9375, 100.3125, 72.9992] | |- IH: [120.9375, 121.875, 143.9985]

Should we include these captures in the tutorial? Should we include or propose these coordinates in the tutorial?

Things to consider:

All in all, increasing accuracy is always a good thing. Don't know what do you guys think. Perhaps the difference is not that significant.

ftadel commented 3 years ago

SimNIBS: Remeshing the SimNIBS model with iso2mesh doesn't work. Here is the error message:

Run through the simnibs part myself and everything seems to be working.

The SimNIBS process works perfectly. What I couldn't get to work is the extraction of the surfaces + remeshing with iso2mesh: https://neuroimage.usc.edu/brainstorm/Tutorials/FemMedianNerve#Remesh_with_Iso2mesh

If this works for you, we need to understand the differences between our two setups.

With this information my results locate the fiducials at these locations: Should we include these captures in the tutorial? Should we include or propose these coordinates in the tutorial?

In the tutorial I used as a starting point, it was requested to "Refine the registration with head points". This makes accurate localization of the fiducials useless. => "Because we will use the digitized head shape of the subject to refine the MRI/MEG registration, we don't need the position of the fiducials to be very accurate."

If you prefer using accurate fiducial positions, we need to disable the automatic registration, and add several pages of instructions to the tutorial to position correctly these fiducials (including the screen capture and positions you shared, description of how you defined these points, etc.)

Last option: we include lots of information about the registration (accurate fiducials) just for teaching purposes, AND discard it all by running the automatic registration. However, the main topic of this tutorial is not coregistration, therefore I'd rather avoid adding sub-branches of discussions that are not related with the topic of interest - this would end-up being only "noise" for people interested primarily in FEM modeling. Coregistration is discussed in earlier tutorials, that the user already read...

juangpc commented 3 years ago

the main topic of this tutorial is not coregistration, therefore I'd rather avoid adding sub-branches of discussions that are not related with the topic of interest

agree. Lets leave it as is.

I'll try to have a look at the simnibs/iso2mesh things later. Thanks,

tmedani commented 3 years ago

agree. Lets leave it as is.

I Agree , let keep it simple.

tmedani commented 3 years ago

Hi Francois, I started reviewing all these points one by one. Some figures can be different indeed since I have tried many fif files and I did not update the images yet.

Can I start editing the tuto or are you still working on it?

juangpc commented 3 years ago

Regarding iso2mesh remeshing: Reviewing the code, it fails here: system() returns a floating point value which is quite strange actually. Not sure if iso2mesh people know about this.

Anyhow, I've tried different combination of surfaces and the remeshing works ok pretty much with any combination of them other than using them all. So I think it is worth trying to modify slightly simnibs values. I'm testing right now. I'll keep posting.

ftadel commented 3 years ago

Some figures can be different indeed since I have tried many fif files and I did not update the images yet. Can I start editing the tuto or are you still working on it?

I am mostly done with all the tutorials: Duneuro, Median nerve, Tensors. You can go ahead and edit them.

I'll work today on the FemMesh tutorial. I hope I'll be done before you start your workday :)

ftadel commented 3 years ago

@fangq We're having issues remeshing with iso2mesh the meshes generated by SimNIBS: https://neuroimage.usc.edu/brainstorm/Tutorials/FemMedianNerve#Remesh_with_Iso2mesh Error reported in the first post, and documented along the thread. In case you have suggestions...

FYI We have just released a tutorial based on Brain2mesh: https://neuroimage.usc.edu/brainstorm/Tutorials/FemTensors#FEM_mesh (please wait for a few days before reviewing the linked tutorial named "FEM mesh generation", what is online is an old draft that I'll update soon)

juangpc commented 3 years ago

Hi @fangq, The error we're experiencing is puzzling. Let me give a bit of info.

We have a pair of coregistered T1 and T2 images which we mesh with SimNIBS . We want to remesh them with Iso2mesh. For that we extract the surfaces of whyte, gray, csf, skull and scalp. And we try to call Iso2mesh to remesh them following the tutorial ftadel is mentioning.

I see a problem in a system() call but it might not be related to our actual issue. I'll explain.

When calling Iso2mesh, this call to system() returns status as a floating point value (-1.0737e+09), instead of an integer which is weird, and the cmdout is empty. Since the execution of tetgen call is defectuous, appart from the error check, readtetgen also fails a few lines after and thus surf2mesh fails.

There are quite a few questions in Matlabcentral regarding system() not working properly showing a behavior very similar to what I see in surf2mesh (like here). (or see... 1 2 3 4 ...). Apparently, system() modifies the path variable of the terminal it opens which can lead to collision of different binary files being executed. However, in surf2mesh the system call to tetgen includes the executable's full path, so the environment variable shouldn't affect. I've also checked with dependency walker and there doesn't seem to be a dll wrongly linked (some people have reported that libiomp5md.dll can cause problems, and in deed I found it in Anaconda, in SimNIBS and in my own path, however I couldn't solve any issues). So maybe this is not the cause of our problems.

We call mergemesh here with the surfaces and it runs great (the bemMerge cell can be downloaded here). A few lines after that, when calling surf2mesh here it fails. However if we repeat the process but selecting different combinations of two, three or four of the 5 surfaces it works. So, like I mentioned at the beginning, I don't know if these two problems are related but the behavior is weird. Plenty of free memory on my system.

Hope it helps.

tmedani commented 3 years ago

Hi @ftadel @juangpc ,

I'm investigating this issue regarding the re-mesh with iso2mesh, it was working a few weeks ago, I'm checking what is changed since then.

It seems to be related to version of the tetgen, the last argument in the sur2mesh function, 'tetgen1.5' , but I'm not sure 100%, something else is happening.

I will give you update asap.

Thanks

fangq commented 3 years ago

@ftadel @juangpc and @tmedani, the status checking was added recently - I haven't encountered strange status code like this before, but happy to take a look.

can you set a break-point at the system call, evaluate the string parameter feed to system, which is basically a shell command, copy paste that command in a terminal and run it, what do you see?

if it still looks strange, please send me all input variables and full command to that surf2mesh call and I will take a look.

juangpc commented 3 years ago

can you set a break-point at the system call, evaluate the string parameter feed to system, which is basically a shell command, copy paste that command in a terminal and run it, what do you see?

If you do that, the command executes with normal output (not empty) and exit status (not floating point).

 "C:\Users\<<username>>\.brainstorm\plugins\iso2mesh\iso2mesh\bin\tetgen1.5.exe" -A -q1.414a1e-08  "C:\Users\<<username>>\.brainstorm\tmp\post_vmesh.poly"

This is the output:

Opening C:\Users\j\.brainstorm\tmp\post_vmesh.poly.
Delaunizing vertices...
Delaunay seconds:  3.526
Creating surface mesh ...
Surface mesh seconds:  0.669
Constrained Delaunay...

However the readtetgen call that attempts to read the result of tetgen still fails.

fangq commented 3 years ago

the above log looks like incomplete - likely tetgen crashed in the middle, can you type echo $? and see if it has a non-zero return value?

my suspicion is that this is related to 32bit/64bit executable issue, which was fixed last year: https://github.com/fangq/iso2mesh/issues/11

if you run iso2mesh on a 64bit windows machine, the tetgen executable (mcpath('tetgen1.5')) s2m runs should be tetgen1.5_x86-64.exe, instead of tetgen1.5.exe.

the part that handles the exe search fallback is here https://github.com/fangq/iso2mesh/blob/master/mcpath.m#L46-L52

PS: this issue was also reported previously here: https://github.com/fangq/iso2mesh/issues/43

fangq commented 3 years ago

on a side note, I'd like to understand the rationales for a user to choose the combination of SimNIBS+iso2mesh remeshing - what is the remeshing step trying to achieve?

on a related question - from the tutorial SimNIBS meshing can take 4-5 hours and the mesh is very dense, why not use brain2mesh? any issue for brain2mesh's outputs? I am asking because I want to see what aspect that brain2mesh can be further improved (at least matching the output quality of SimNIBS), while giving full control on mesh density.

juangpc commented 3 years ago

the above log looks like incomplete - likely tetgen crashed in the middle, can you type echo $? and see if it has a non-zero return value?

I get no return value. Not sure if it is my terminal though.

if you run iso2mesh on a 64bit windows machine, the tetgen executable (mcpath('tetgen1.5')) s2m runs should be tetgen1.5_x86-64.exe, instead of tetgen1.5.exe.

Could it be that we're downloading an outdated version? tetgen1.5_x86-64.exe is not present in the release v1.9.2. And the mcpath.m Brainstorm users are running doesn't search for x64 executables.

Brainstorm downloads from Iso2mesh's releases the following one: https://github.com/fangq/iso2mesh/releases/tag/v1.9.2 Which was generated on Feb 13 2020. The part that handles 64bit exe was committed a week later https://github.com/fangq/iso2mesh/commit/ddc9a16ba4a1186e16ca449b70f83441eaa7989a

juangpc commented 3 years ago

If I manually clone Iso2mesh's updated repo and install it for Brainstorm to use it. I'm able to get a different output. system() seems to be now working fine. So it probably was that x32, x64 confusion. Thank you for that.

The error I get now is:

image

I can remesh no problem all surfaces except the scalp. So it seems there might be some problem there.

juangpc commented 3 years ago

any issue for brain2mesh's outputs?

Isn't it necessary to have matlab's Image processing toolbox installed?

tmedani commented 3 years ago

Hi @fangq and thanks for your feedback and advice.

PS: this issue was also reported previously here: fangq/iso2mesh#43

indeed we have discussed this issue some time ago but it appears in some cases that I'm also investigating.

on a side note, I'd like to understand the rationales for a user to choose the combination of SimNIBS+iso2mesh remeshing - what is the remeshing step trying to achieve?

This is related to two main reasons,

ftadel commented 3 years ago

Brainstorm downloads from Iso2mesh's releases the following one: https://github.com/fangq/iso2mesh/releases/tag/v1.9.2 Which was generated on Feb 13 2020. The part that handles 64bit exe was committed a week later fangq/iso2mesh@ddc9a16

@fangq Could you recompile a newer version, as you did last year? We'd only need the "-allinone.zip" package. Thanks!

fangq commented 3 years ago

Re @ftadel: @fangq Could you recompile a newer version, as you did last year? We'd only need the "-allinone.zip" package.

done, see https://github.com/fangq/iso2mesh/releases/tag/v1.9.6

Re @juangpc: I can remesh no problem all surfaces except the scalp

from the error message, looks like the input surface contains self-intersections, there must be something in the earlier steps that has caused this. please check

Re @juangpc: Isn't it necessary to have matlab's Image processing toolbox installed?

indeed - the only thing I need are imdilate/imfill, see fangq/brain2mesh#10, still haven't located a portable replacement

The first is that for the FEM solver of the EEG/MEG leadfield with the Duneuro fails in some cases,

did you mean brain2mesh created surfaces have holes? that would be unexpected, I would love to get a reproducer and test

it's not important to keep the outer layers in the simulation, we build the model only by using the inner tissues (wm, gm and csf)

in brain2mesh, only wm/gm are required, all other segments (csf, bone, scalp) are optional, see

https://github.com/fangq/brain2mesh/commit/3e88bd867bb1e84e11915388799735a14d7fc91c https://github.com/fangq/brain2mesh/blob/master/brain2mesh.m#L32

again, I am aware of the output quality differences between SimNIBS and SPM seg derived brain2mesh meshes, just want to if there is anything easy to fix

ftadel commented 3 years ago

@fangq: done, see: https://github.com/fangq/iso2mesh/releases/tag/v1.9.6

Thanks! I updated the link in Brainstorm (this will update iso2mesh automatically after Brainstorm is updated): https://github.com/brainstorm-tools/brainstorm3/commit/96f149434b1b362bf3f2df91505d862708d6657f

@juangpc @tmedani It solves the remeshing issue with the tutorial! (at least on my computer) https://neuroimage.usc.edu/brainstorm/Tutorials/FemMedianNerve

@fangq: did you mean brain2mesh created surfaces have holes? that would be unexpected, I would love to get a reproducer and test

No, brain2mesh fills entirely the head volume. The "problem" is related only with SimNIBS, which doesn't mesh some of the air cavities, causing DUNEuro to crash in some cases (I haven't seen this myself, in the current tutorials, it works).

SimNIBS (air cavities in the anterior part of the skull): image

Surfaces extracted and remeshed with Iso2mesh (no more holes): image

@fangq: again, I am aware of the output quality differences between SimNIBS and SPM seg derived brain2mesh meshes, just want to if there is anything easy to fix

I can't judge the quality of the tetrahedral mesh.

From a more general point of view in Brainstorm source estimation: SimNIBS has the advantage of running a full CAT12 pipeline, which generates good quality triangular meshes of the grey matter. However, we can run an updated version of CAT12 as a Brainstorm plugin, with more tailored options for generating anatomical atlases. Therefore SimNIBS has no real advantage, as we have an option for a 100% Matlab/Brainstorm-based pipeline, which runs faster and with better-quality similar pial triangular meshes.

@juangpc @tmedani Maybe you can elaborate on the choice of recommending SimNIBS over Brain2mesh+CAT12?

tmedani commented 3 years ago

Hi all, Sorry for being slow in this discussion, I was on vacation, and then had a lot of emails and accumulated deadlines.

Thanks, @fangq, and @ftadel for solving the meshing issues

@juangpc @tmedani It solves the remeshing issue with the tutorial! (at least on my computer) https://neuroimage.usc.edu/brainstorm/Tutorials/FemMedianNerve

I will check it on my computer as well asap.

The "problem" is related only with SimNIBS, which doesn't mesh some of the air cavities, causing DUNEuro to crash in some cases (I haven't seen this myself, in the current tutorials, it works).

@ftadel do you mean that Duneuro works even on the model with holes? for both modalities EEG/MEG? this is interesting to investigate because it was not working on my case for MEG. I have already reported this issue to the Duneuro dev team.

@juangpc @tmedani Maybe you can elaborate on the choice of recommending SimNIBS over Brain2mesh+CAT12?

We have already integrated brain2mesh within brainstorm and tested in some case, however since brain2mesh need a segmented volume as an input, we need first to segment the MRI, and from our testings, we have observed bad segmentation outputs in some cases which lead to a bad mesh.

However since simnibs has the full process with integrated methods that correct the segmentation, so in most cases, it has a better mesh, this was the main reason why we have recommended SimNibs for now.

So, if we have a robust method for segmenting the whole, then of course brain2mesh is the straightforward method to recommend since it's Matlab based, and it can be much faster.

ftadel commented 3 years ago

do you mean that Duneuro works even on the model with holes? for both modalities EEG/MEG? this is interesting to investigate because it was not working on my case for MEG.

Actually, I realize that I haven't let the MEG FEM run on my old laptop until the end, so I don't know... EEG works, MEG maybe not. If this is only the MEG that crashes, could you please add this information (after testing it) in the tutorial, in the section about the iso2mesh remeshing? Thanks

ftadel commented 2 years ago

@tmedani
Is there anything left to do from this issue?

tmedani commented 2 years ago

Hi @ftadel,

Thanks for checking on that. yes, there are some remaining points, and then I need to update the tutorial.

I jumped to something else, but I will be back to that and complete it.