Skip to content

Add Polygon/Polyhedron NSIDED/NFACED ExodusII output#4490

Open
ConnorOuellette wants to merge 2 commits into
libMesh:develfrom
ConnorOuellette:c0polyhedron-exodus-nfaced
Open

Add Polygon/Polyhedron NSIDED/NFACED ExodusII output#4490
ConnorOuellette wants to merge 2 commits into
libMesh:develfrom
ConnorOuellette:c0polyhedron-exodus-nfaced

Conversation

@ConnorOuellette

Copy link
Copy Markdown

Summary

This adds ExodusII output support for libMesh’s arbitrary polygon and polyhedron elements:

  • writes C0POLYGON elements as Exodus NSIDED blocks
  • writes C0POLYHEDRON elements as Exodus NFACED blocks
  • emits the associated face block, face-node connectivity, face counts, element-face connectivity, and element face counts needed for NFACED output

Notes

  • If using ParaView, the default IOSS Reader does not support polygonal/polyhedral input. Please use the Legacy Exodus Reader.
  • Fixed an error with build_cube() where corner nodes were scaled twice

@roystgnr roystgnr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd have split this into like 6 commits instead of 2 (re-bootstrap should be separate, tests could be separate, additional build_cube option should be separate), but the contents look mostly solid. That hardcoded block id (unless I'm just misunderstanding?) is the only thing that looks wrong.

num_face_blk = 0;

dof_id_type local_num_c0polyhedron_faces = 0;
int has_c0polyhedron = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's make this a bool and use false/true, for clarity? You can still max() it with TIMPI.

Comment on lines +90 to +91
int num_face_blk;
int num_face;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These additions look like The Right Way To Do It to me, but I'll want to give @jwpeterson time to chime in before merging; IIRC the ExodusHeaderInfo refactor was an addition of his for Akselos usage.


using namespace libMesh;

constexpr int c0polyhedron_face_block_id = 1;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we hard-coding a block id here? That definitely needs to be fixed. It'll break as soon as someone tries to output a mesh that already has that id as a subdomain id.

There's a build_subdomain_map that we use to handle this problem when we've been requested to output side elements for visualization, and it might be what we want to extend for this side output too, whether because we need it or just because we really want all this hackery to be in one place so that the different optional features don't stomp on each other's toes.

<< " in the same block!");

for (unsigned int j=0; j<static_cast<unsigned int>(num_nodes_per_elem); ++j)
if (is_c0polygon_block)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The swath of if/else if/else here seems like it's leading to a lot of duplicated lines; could we use a lambda function or something to cut them down?

const auto dx_tri = (xmax - xmin) / (nx);
const auto dy_tri = (ymax - ymin) / (ny + 1);
const auto dx_tri = Real(1) / nx;
const auto dy_tri = Real(1) / (ny + 1);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I want to whine that the mesh generation fixes should have been in a separate commit, but the important thing is that the bug got fixed; thanks!

mesh.reserve_nodes(grid_nodes +
(type == C0POLYHEDRON ?
cast_int<dof_id_type>(nx*ny*nz) :
0));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The extra entries here cover the mid-polyhedron nodes that we generate to keep internal tetrahedralization robust? Let's put that in a separate mid_polyhedron_nodes variable and add a comment explaining it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants