Error when calling the "mergesurf"

[tmedani]

Hi @fangq

Regarding our discussion about the error on "mergesurf" the discussion was started here: https://github.com/brainstorm-tools/brainstorm3/issues/185#issuecomment-588283689

Here is the error :

K>> [newnode, newelem] = mergesurf(bemMerge{:});
Error using surfboolean (line 188)
surface boolean command failed:
cd "C:\Users\33649\AppData\Local\Temp\iso2mesh-medani\" &&
"C:\Users\33649\.brainstorm\iso2mesh\win64\iso2mesh\bin\cork.exe" -resolve
"C:\Users\33649\AppData\Local\Temp\iso2mesh-medani\pre_surfbool1.off"
"C:\Users\33649\AppData\Local\Temp\iso2mesh-medani\pre_surfbool2.off"
"C:\Users\33649\AppData\Local\Temp\iso2mesh-medani\post_surfbool.off" -1648335518
ERROR:

Error in mergesurf (line 39)
   [newnode,newelem]=surfboolean(newnode,newelem,'all',no,el);

Here is the link to the bemMergedata used in this function : https://www.dropbox.com/transfer/AAAAAKGCEsm9-ZjHkyNhHVC8U6-WpakkIL17e3RlXGEDPB25Gn00P5s

We are sure that these surfaces are not intersecting since they are extracted from a known tetrahedral mesh. Here is a view of the surfaces:

and the tetra mesh from where they are extracted:

In the meantime, I will try to reproduce it on Linux to check your proposition.

Thank you. Takfarinas

[fangq]

@tmedani, I tested your input data on Linux, it worked fine.

I suppose this is a problem with windows only. I will look into this further.

If, however, your algorithm guarantee that these surfaces do not intersect to each other, you do not have to use mergesurf, use mergemesh. If they actually intersect, surf2mesh will complain after mergemesh.

[tmedani]

@fangq Thanks for your answer and checking on linux.

If, however, your algorithm guarantee that these surfaces do not intersect to each other, you do not have to use mergesurf, use mergemesh. If they actually intersect, surf2mesh will complain after mergemesh.

indeed, we have added an option that asks the user for the choice of the method.

Also, we noticed that when "surf2mesh" complains, it loads the previous correct mesh saved on the temporary folder.

This could introduce confusion for the users. Maybe it's better to stop the execution, I will check this as well asap.

Best, Takfarinas

[fangq]

Also, we noticed that when "surf2mesh" complains, it loads the previous correct mesh saved on the temporary folder.

thanks for reporting this - I noticed this occasionally, but never took the opportunity to fix it. with this patch, it should be fixed

https://github.com/fangq/iso2mesh/commit/8454bf61e1cca9df03a91cb81ee69f5290b958eb

[fangq]

I just changed from isempty to ~isempty in deletemeshfile

[lrineau]

We are sure that these surfaces are not intersecting since they are extracted from a known tetrahedral mesh.

Even if the triangles of the surfaces cannot intersect in their interior, the surfaces could intersect in the sense that they could "touch" each other, sharing edges or vertices.

[fangq]

@lrineau, that is true. however, one can call

meshcheckrepair(no,fc,'dup')

or

fc=removedupelem(fc)

in iso2mesh to remove duplicated triangles in a combined mesh.

I believe tetgen also has a builtin step to remove duplicated triangles, so even without the above step, surf2mesh/s2m should also produce a valid tet mesh.

We are sure that these surfaces are not intersecting since they are extracted from a known tetrahedral mesh.

@ftadel, if this is the case, I would say there is no benefit to call mergesurf, simply call mergemesh and then meshcheckrepair(no,fc,'dup'), that should be fast and error free.

[fangq]

@ftadel, after testing this further, I think the failed surfboolean call was a result of memory allocation error for a 32bit binary on windows.

on my windows desktop (ryzen 1700x, 16GB, 64bit windows 10), I was able to run

[newnode, newelem] = mergesurf(bemMerge{1:8});

or

[newnode, newelem] = mergesurf(bemMerge{3:end});

separately, but when merging all 5 surfaces, cork.exe got killed. I think this is a typical behavior when a 32bit binary allocates big memory that exceeds a limit (4GB in theory, but I've seen this error with much less size).

I plan to close out this ticket, and link this to Issue #11. Providing 64bit windows binaries (including CGAL, tetgen and meshfix) should solve this problem.

let me know if this sounds ok to you.

[tmedani]

@lrineau Thanks for the precisions. @fangq we are using mergemesh as the default option, in the previous example (https://github.com/fangq/iso2mesh/issues/43#issue-567833751) we are sure that the surfaces are not intersecting, but it's not always the case.

We have added the mergesurf for specific/advanced cases.

[fangq]

@tmedani, I compiled a copy of 64bit cork and updated mcpath.m to allow co-existence of 32bit and 64bit windows exe.

with the updated 64bit cork, I was able to run the mergesurf() command with all 5 surfaces - I did not wait it to complete, but running for about 10 min without seeing any crash or error. It consumed about 10GB of memory on my windows server before I killed the command. I am pretty confident the previous error was the missing of 64bit support, although I don't fully understand why merging 4 surfaces only consumes <1GB memory but 5 surfaces requires such a large memory need.

any how, give it a try, if it does not work, feel free to reopen this ticket. I will also work on #11 and add all 64bit binaries for windows.

[fangq]

here are the related cork code changes I made in order to compile it on cygwin64

https://github.com/fangq/cork/commit/3597f9f71a5be64dfaffa65879441032dbcaef54

the linking command is done manually with -static-libgcc -static-libstdc++ to shave a few dlls, but I could not get rid of libgmp-10.dll

[ftadel]

if this is the case, I would say there is no benefit to call mergesurf, simply call mergemesh and then meshcheckrepair(no,fc,'dup'), that should be fast and error free.

@tmedani I added a call to meshcheckrepair after merge mesh then. Ok for you? https://github.com/brainstorm-tools/brainstorm3/commit/4b016f702ad0856acc68f878104112cba60692a6

@fangq I'll update the Brainstorm distribution only with new github releases of iso2mesh. Could you create a new release when you have this 64bit windows functions ready? (or a new win64 package for the same release?)

[fangq]

@ftadel, please replace the meshcheckrepair() line you added to

newelem=unique(sort(newelem,2),'rows');

if for some reason, you prefer to keep the original node index orders in newelem instead of sorted, you can use the below two lines

[tmpelem,idx]=unique(sort(newelem,2),'rows');
newelem=newelem(idx,:);

here is why:

I realized that meshcheckrepair('dup') is designed to remove doubly-duplicated elements, i.e., if a triangle repeats itself by even numbers (such as internal boundaries), then it will be removed. If you want to simply remove duplicated elements and keep only 1 copy, then, you should use the unique call.

to show this, here is a test:

% first, create two boxes intersecting each other
[n1,f1]=meshabox([0,0,0], [10,10,10], 1, 10);
[n2,f2]=meshabox([3,3,-1], [7,7,4], 1, 10);

% orient both surfaces to be counter-clockwise node order
[n1,f1]=surfreorient(n1,f1);
[n2,f2]=surfreorient(n2,f2);

% do a boolean, keep the subsurfaces within the first mesh (bigger box)
[n3,f3]=surfboolean(n1,f1,'first',n2,f2(:,[1 3 2]));

% see the resulting surface - two compartments
figure;
subplot(221); plotmesh(n3,f3,'x>5')
title('surfboolean output');

% create a tet mesh, seed two regions
[node,elem]=s2m(n3,f3,1,10,'tetgen',[1 1 1; 5 5 1]);
subplot(222); plotmesh(node,elem,'x>5')
title('initial meshing');

% extract the surfaces of the outer & inner box surfaces - inner bottom surface are duplicated!
fc1=volface(elem(:,1:4));
fc2=volface(elem(elem(:,end)==2,1:4));

% combine two surfaces, with duplicates, then remove interior nodes, to form a new surf
[newn1,newf1]=removeisolatednode(node,[fc1; fc2]);

% because newf1 has duplicates, the below call will fail s2m/tetgen
% [node2,elem2]=s2m(newn1,newf1,1,10,'tetgen',[1 1 1; 5 5 1]);

% if you call meshcheckrepair('dup'), the shared surface is gone
[newn2,newf2]=meshcheckrepair(newn1,newf1,'dup');
subplot(223);  plotmesh(newn2,newf2,'x>5');
title('meshcheckrepair remove the shared surf');

% tetgen will work but you are now missing the inner box
% [node2,elem2]=s2m(newn2,newf2,1,10,'tetgen',[1 1 1; 5 5 1]);

% however, if you call unique to remove duplicates, this will keep one copy
newf1=unique(sort(newf1,2),'rows');
subplot(224); plotmesh(newn1,newf1,'x>5')
title('unique does not remove the shared surf');

[node2,elem2]=s2m(newn1,newf1,1,10,'tetgen',[1 1 1; 5 5 1]);

I will add a new option to meshcheckrepair and removedupelem, say 'unique', to achieve this moving forward.

[fangq]

@fangq I'll update the Brainstorm distribution only with new github releases of iso2mesh. Could you create a new release when you have this 64bit windows functions ready? (or a new win64 package for the same release?)

will do. I am spending this weekend on adding new functions to brain2mesh to prepare for a new release

https://github.com/fangq/brain2mesh/commits/master

will likely absorb some of these new utilities to iso2mesh. I will create a new release once this is done. if you guys have any release deadlines, let me know.

[ftadel]

please replace the meshcheckrepair() line you added to newelem=unique(sort(newelem,2),'rows');

Done: https://github.com/brainstorm-tools/brainstorm3/commit/936c21b9972d5a09c844c29ef21ef880835760eb

if you guys have any release deadlines, let me know.

We'd like to have something ready by the end of the spring, no rush.

Thanks!