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

Increased diagnostic output and added a few lines of documentation #238

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

mpotse
Copy link
Contributor

@mpotse mpotse commented Dec 31, 2023

Increased diagnostic output and added a few lines of documentation.

  • print all coquilface warnings
  • print problem locations (coordinates)
  • min mesh quality in %g format

…ilface warnings, print problem location, min mesh quality in %g format, a few lines of documentation
@CLAassistant
Copy link

CLAassistant commented Dec 31, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@Algiane Algiane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

Thanks for this contribution.
I would prefer to increase the verbosity of Mmg only in debug mode if you agree.

Best

Comment on lines +1890 to +1891
MMG5_show_tet_location(mesh, pt, start);
if(0) mmgWarn0 = 1; // disabled, I want to see how many there are!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it is not obvious so I prefer to explain this a little: the number of times the message is displayed does not reflect the number of edges for which an error is detected.

The function that may raise this warning is called only:

  1. when attempting to swap a boundary edge;
  2. when attempting to split such an edge.

In consequence:

  • It is possible to have an erroneous edge without seeing this warning;
  • a single erroneous edge may raise lot of warnings because:
    • this edge may be seen by the swapping and/or splitting operator;
    • the edge will probably be seen during multiple remeshing iterations;
    • at each iteration and for each operator we will see the edge from all the elements to which it belongs (and as the edge is not modified, we will try to do something and fail each time we will see it);

As the number of warning that is printed is not really meaningful, and as lot of users doesn't need to have the message more than once, I propose to keep this proposition of higher level of verbosity only in debug mode (-d command line option of Mmg):

if ( mesh->info.ddebug ) {
  // print indices and coordinates of the vertices of one tet.
  MMG5_show_tet_location(mesh, pt, start);
} else {
  // disable warnings for this error as the message has already been printed once
  mmgWarn0 = 1;  
} 

Comment on lines 542 to 562



/* -------------------------------------- Mark's hacks --------------------------------------------- */

// Print the indices and coordinates of the vertices of one tet. This is to
// inform the user about the location of a problem when the tet index is not
// helpful, for example because it does not correpond to the input or output
// mesh.
//
void MMG5_show_tet_location(MMG5_pMesh mesh, MMG5_pTetra pt, int iel)
{
fprintf(stderr, " ## tet index %d\n", iel);
for(int j=0; j<4; j++){
double U[3], *S = mesh->point[pt->v[j]].c; // unscaled and scaled coords
for(int i=0; i<3; i++) U[i] = S[i]*mesh->info.delta + mesh->info.min[i];
fprintf(stderr, " ## vertex %d at (%f,%f,%f)\n", pt->v[j], U[0], U[1], U[2]);
}
}


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it can be useful.

Just for your information, we also have debugging tools to print the packed indices of tetra and points (which means the indices they will have in the output mesh):

  • the MMG3D_indElt(mesh,k) function gives you the index of tetra k inside the output mesh;
  • the MMG3D_indPt(mesh,k) function gives you the index of point k inside the output mesh;

Note that it is useful and trustable only if you save the mesh immediately after calling these functions (because tetra and points indices will be modified if we continue the remeshing so, even if the targetted point/tetra are not touched, the mesh packing will be different).

@@ -625,16 +625,24 @@ MMG5_int MMG5_swpmsh(MMG5_pMesh mesh,MMG5_pSol met,MMG3D_pPROctree PROctree, int

ret = MMG5_coquilface(mesh,k,i,ia,list,&it1,&it2,0);
ilist = ret / 2;
if ( ret < 0 ) return -1;
if ( ret < 0 ){
MMG5_show_tet_location(mesh, pt, k);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, keep this in debug mode only (-d command line option, mesh->info.ddebug variable inside the code) :

if ( mesh->info.ddebug ) {
   MMG5_show_tet_location(mesh, pt, k);
}

Comment on lines +635 to +636
if ( ier < 0 ){
MMG5_show_tet_location(mesh, pt, k);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, keep this in debug mode only (-d command line option, mesh->info.ddebug variable inside the code) :

if ( mesh->info.ddebug ) {
   MMG5_show_tet_location(mesh, pt, k);
}

else if ( ier ) {
ier = MMG5_swpbdy(mesh,met,list,ret,it1,PROctree,typchk);
if ( ier > 0 ) ns++;
else if ( ier < 0 ) return -1;
else if ( ier < 0 ){
MMG5_show_tet_location(mesh, pt, k);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, keep this in debug mode only (-d command line option, mesh->info.ddebug variable inside the code) :

if ( mesh->info.ddebug ) {
   MMG5_show_tet_location(mesh, pt, k);
}

}
}
else {
if (MMG5_boulesurfvolp(mesh,k,ip,i,
list,&ilist,lists,&ilists,p0->tag & MG_NOM) < 0 )
list,&ilist,lists,&ilists,p0->tag & MG_NOM) < 0 ){
MMG5_show_tet_location(mesh, pt, k);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, keep this in debug mode only (-d command line option, mesh->info.ddebug variable inside the code) :

if ( mesh->info.ddebug ) {
   MMG5_show_tet_location(mesh, pt, k);
}

@@ -1134,13 +1166,19 @@ static int MMG5_coltet(MMG5_pMesh mesh,MMG5_pSol met,int8_t typchk) {

if ( ilist > 0 ) {
ier = MMG5_colver(mesh,met,list,ilist,iq,typchk);
if ( ier < 0 ) return -1;
if ( ier < 0 ){
MMG5_show_tet_location(mesh, pt, k);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, keep this in debug mode only (-d command line option, mesh->info.ddebug variable inside the code) :

if ( mesh->info.ddebug ) {
   MMG5_show_tet_location(mesh, pt, k);
}

else if ( ier ) {
MMG3D_delPt(mesh,ier);
break;
}
}
else if (ilist < 0 ) return -1;
else if (ilist < 0 ){
MMG5_show_tet_location(mesh, pt, k);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, keep this in debug mode only (-d command line option, mesh->info.ddebug variable inside the code) :

if ( mesh->info.ddebug ) {
   MMG5_show_tet_location(mesh, pt, k);
}

@@ -2593,6 +2631,7 @@ MMG3D_anatets_iso(MMG5_pMesh mesh,MMG5_pSol met,int8_t typchk) {
fprintf(stderr,"\n ## Warning: %s: surfacic pattern: unable to find"
" a valid split for at least 1 point. Point(s) deletion.\n",
__func__ );
MMG5_show_tet_location(mesh, pt, k);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, keep this in debug mode only (-d command line option, mesh->info.ddebug variable inside the code) :

if ( mesh->info.ddebug ) {
   MMG5_show_tet_location(mesh, pt, k);
}

@@ -3188,7 +3227,7 @@ int MMG5_anatet(MMG5_pMesh mesh,MMG5_pSol met,int8_t typchk, int patternMode) {
if ( !mesh->info.noinsert ) {
nc = MMG5_coltet(mesh,met,typchk);
if ( nc < 0 ) {
fprintf(stderr,"\n ## Unable to collapse mesh. Exiting.\n");
fprintf(stderr,"\n ## Unable to collapse mesh in MMG5_anatet: nc=%d. Exiting.\n", nc);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we pass here, nc is equal to -1 no ?

…next tet instead of returning to the caller.

Also changed more return values so I can see where errors come from.
distinguish harmful from harmless returns from  MMG5_boulesurfvolpNom()
…that it is also available in the library version
@Algiane
Copy link
Member

Algiane commented Mar 4, 2024

Hi @mpotse ,
Let me know when this PR is ok for you so I can review it again.
Best

@Algiane Algiane added kind: enhancement enhancement to an existing feature kind: cleanup should be clean part: mmg3d mmg3d specific labels Apr 4, 2024
@Algiane Algiane self-assigned this Apr 4, 2024
mpotse added 3 commits May 10, 2024 09:06
  This reverses one line of commit ba87623
  where it was argued that this is a "parallel tag" which is to be removed
  from faces that are not parallel faces. It may have been necessary for
  ParMmg but it erases all RequiredTriangles from mmg3d output, and
  I rely on them in the "mmg3d -ls ..." calls in WP7/synthetic/mason.sh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: cleanup should be clean kind: enhancement enhancement to an existing feature part: mmg3d mmg3d specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants