Skip to content

Conversation

@jwpeterson
Copy link
Member

@jwpeterson jwpeterson commented Nov 4, 2025

The basic idea here is that the Elems and Nodes will always be numbered sequentially starting from zero in libmesh while their "true" ids are stored as their unique_id (this idea is from @roystgnr). The idea behind this option is to better support non-contiguous numberings by simply using a contiguous numbering within libmesh and maintaining the {node,elem}_num_maps on the Elems and Nodes themselves until the information is written back to file. There will likely be a lot of follow-up work on this, but this PR adds the option (off by default) so that I can start experimenting further with it.

Fix missing initialization of this flag in ExodusII_IO.
…() to support _set_unique_ids_from_maps flag
Although in this case both types in the tuple are the same, this
syntax allows you to define local variables with different types
directly in the for-loop clause.
Also, add an error check to the get_libmesh_node_id() function so that
we get a nice error message rather than accessing past the end of the
node_num_map vector.
The helper function effectively does nothing in the case where
set_unique_ids_from_maps == true, since in that case the libmesh id is
just a (zero-based) version of the input. We then determine the
unique_id after the new Node has already been added, since that is
when it is most convenient to set the unique_id value.
Previously, we just wrote an identity vector for the node_num_map, but
now we populate it with the Node unique_ids if the user sets the
_set_unique_ids_from_maps boolean.
@moosebuild
Copy link

Job Test mac on d1ffb61 : invalidated by @jwpeterson

Failed to connect to conda server

@jwpeterson
Copy link
Member Author

The "Test MOOSE GCC min" failures should be completely unrelated.

[300.0s]  TIMEOUT stochastic_tools/test:web_server_control.stochastic_control/batch_reset FAILED (TIMEOUT)
[1.159s]     FAIL phase_field/tutorials/spinodal_decomposition.s2_fasttest FAILED (CRASH)

@moosebuild
Copy link

Job Coverage, step Generate coverage on fc30535 wanted to post the following:

Coverage

af134b #4308 fc3053
Total Total +/- New
Rate 65.00% 65.02% +0.01% 96.05%
Hits 76964 76998 +34 73
Misses 41434 41433 -1 3

Diff coverage report

Full coverage report

This comment will be updated on new commits.

@moosebuild
Copy link

Job Test MOOSE GCC min on fc30535 : invalidated by @jwpeterson

See if unrelated failures go away

Copy link
Member

@roystgnr roystgnr left a comment

Choose a reason for hiding this comment

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

I'm not seeing any serious issues, but I must admit that there were half a dozen spots here where I found myself thinking "I sure wish we had better test coverage for that" - ExodusII_IO is such a stew of weird optional features at this point.

* without any gaps in the numbering).
* .) When writing an Exodus file, populate the elem_num_map and
* node_num_map with the unique_ids of the Elems/Nodes being
* written.
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a comment on why one might want to do this? (ReplicatedMesh using O(max_id) storage)

* that flag is false. The input index is assumed to be a zero-based
* index into the {node,elem}_num_map array.
*/
void set_node_unique_id(
Copy link
Member

Choose a reason for hiding this comment

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

I do not like set_foo_unique_id for the name of a method that may set the unique id or may not do anything ... but I can't actually think of a better name, I'm afraid. If you don't have any better ideas we can keep this.

Copy link
Member Author

Choose a reason for hiding this comment

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

possibly_set_node_unique_id()
conditionally_set_node_unique_id()
?

Copy link
Member

Choose a reason for hiding this comment

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

Either of those is an improvement. I like conditionally.

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.

3 participants