Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression for LSReferences in level-set discretization with input domains preservation #150

Open
cbritopacheco opened this issue Jun 28, 2022 · 2 comments
Labels
kind: regression detected regression part: ls discretization isovalue discretiztion mode specificity

Comments

@cbritopacheco
Copy link
Contributor

cbritopacheco commented Jun 28, 2022

Environment

OS macOS Big Sur
Compiler clang version 11.1.0
mmg 5b7cf4e
Build Debug

Problem

Given the files:

mmg.mesh.txt
mmg.sol.txt
mmg.mmg2d.txt

MMG fails using the command:

mmg2d -ls mmg

outputting the message:

  -- MMG2D, Release 5.6.0 (Nov. 05, 2021)
     Copyright (c) Bdx INP/CNRS/Inria/UPMC, 2004-
     Jun 28 2022 16:57:45

  -- INPUT DATA
  %% mmg.mesh OPENED
  %% mmg.sol OPENED

  %% mmg.mmg2d OPENED

   ## Error: Trying to overwrite material reference 1 (from LSReferences line 1) with another entry from LSReferences line 1.
             Check your LSReferences table: each material reference should be unique!

 ## Error: MMG5_Set_multiMat: unable to create lookup table for multiple materials.

   ELAPSED TIME  0.022s

Reason

The contents of the .mmg2d file are the following:

LSReferences
2

1 1 2
2 1 2

At some point (see #112 and 37774f2) this functionality was removed (intentionally or unintentionally I don't know). However, I don't see why it would be removed since it is quite useful to specify that the new materials will retain their original labels. I use this in my code which uses the master branch but I tested it on the develop branch and came up with this problem. Eventually I would like to upgrade hence the reason for this bug report.

Stack trace

MMG5_Set_multiMat leads into the problem via the trace:

(lldb)
frame #4: 0x00000001008f50d8 LevelSetCantilever2D`MMG5_InvMat_set(mesh=0x0000000114604d48, pim=0x0000000114604f40, k=0) at mmg2.c:154:7
   151 	    if( MMG5_InvMat_check(pim,key) ) {
   152 	      pim->lookup[key] = MMG5_InvMat_code(k,MG_MINUS);
   153 	    } else {
-> 154 	      MMG5_InvMat_error(pim,pm->rin,k);
   155 	      return 0;
   156 	    }
   157 	    key = MMG5_InvMat_key(pim,pm->rex);
(lldb)
frame #5: 0x00000001008f4f38 LevelSetCantilever2D`MMG5_MultiMat_init(mesh=0x0000000114604d48) at mmg2.c:366:10
   363
   364 	  /* Fill lookup table */
   365 	  for( k = 0; k < mesh->info.nmat; k++ ) {
-> 366 	    if( !MMG5_InvMat_set(mesh,pim,k) )
   367 	      return 0;
   368 	  }
   369
(lldb)
frame #6: 0x00000001008ef6e0 LevelSetCantilever2D`MMG5_Set_multiMat(mesh=0x0000000114604d48, sol=0x000060000210c0a8, ref=2, split=1, rin=1, rex=2) at libtools.c:137:10
   134
   135 	  /* Invert the table if all materials have been set */
   136 	  if( mesh->info.nmati == mesh->info.nmat )
-> 137 	    if( !MMG5_MultiMat_init(mesh) ) {
   138 	      fprintf(stderr,"\n ## Error: %s: unable to create lookup table for multiple materials.\n",
   139 	              __func__);
   140 	      return 0;
(lldb)
frame #7: 0x00000001008fd4b4 LevelSetCantilever2D`MMG2D_Set_multiMat(mesh=0x0000000114604d48, sol=0x000060000210c0a8, ref=2, split=1, rin=1, rout=2) at API_functions_2d.c:394:10
   391
   392 	int MMG2D_Set_multiMat(MMG5_pMesh mesh,MMG5_pSol sol,int ref,
   393 	                       int split,int rin,int rout){
-> 394 	  return MMG5_Set_multiMat(mesh,sol,ref,split,rin,rout);
   395 	}
   396
   397
@Algiane Algiane added kind: regression detected regression part: ls discretization isovalue discretiztion mode specificity labels Jun 28, 2022
@Algiane
Copy link
Member

Algiane commented Jun 28, 2022

Hello,

Thanks for the detailed bug report: there is indeed a regression in the iso-value discretization option.

In some cases, the non-invertibility of the material map causes problems quite difficult to solve so we have decided (a little too quickly) to impose our users to provide bijective maps... and we didn't see that we were breaking one of the most common use of the multi-material mode.

It is hard to simply revert the involved commit as it will broke the usage of this option for other users. I will try to find the time to look at this as soon as possible.

Best Regards,
Algiane

@Algiane
Copy link
Member

Algiane commented Dec 12, 2022

Hi,

I think that I have fixed this regression: non-bijective maps are authorized again as initially and Mmg now raises only a warning. Just note that I hove done nothing to improve the handling of such maps and this option is to use at your own risks: if possible (but I know that it is not always the case), try to provide a bijective map to Mmg.

Do not hesitate to test the develop branch of Mmg and to close this issue if it works.

Thank you by advance,
Best Regards,

Algiane

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: regression detected regression part: ls discretization isovalue discretiztion mode specificity
Projects
None yet
Development

No branches or pull requests

2 participants