From 0016a5a263fbc7f49a75667de99c3982914813b3 Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Wed, 3 Jan 2024 12:59:24 -0600 Subject: [PATCH 01/15] Make is_prepared more fine-grained --- include/mesh/mesh_base.h | 200 +++++++++++++++++++++++++++++++---- src/mesh/boundary_info.C | 2 + src/mesh/distributed_mesh.C | 8 +- src/mesh/mesh_base.C | 90 +++++++++++----- src/mesh/replicated_mesh.C | 6 +- src/mesh/unstructured_mesh.C | 4 +- 6 files changed, 260 insertions(+), 50 deletions(-) diff --git a/include/mesh/mesh_base.h b/include/mesh/mesh_base.h index b1c40a44ee..c30a559a49 100644 --- a/include/mesh/mesh_base.h +++ b/include/mesh/mesh_base.h @@ -165,7 +165,7 @@ class MeshBase : public ParallelObject virtual ~MeshBase (); /** - * A partitioner to use at each prepare_for_use() + * A partitioner to use at each partitioning */ virtual std::unique_ptr & partitioner() { return _partitioner; } @@ -194,7 +194,7 @@ class MeshBase : public ParallelObject * No Node is removed from the mesh, however even NodeElem elements * are deleted, so the remaining Nodes will be considered "unused" * and cleared unless they are reconnected to new elements before - * the next \p prepare_for_use() + * the next preparation step. * * This does not affect BoundaryInfo data; any boundary information * associated elements should already be cleared. @@ -202,17 +202,105 @@ class MeshBase : public ParallelObject virtual void clear_elems () = 0; /** - * \returns \p true if the mesh has been prepared via a call - * to \p prepare_for_use, \p false otherwise. + * \returns \p true if the mesh has undergone all the preparation + * done in a call to \p prepare_for_use, \p false otherwise. */ bool is_prepared () const - { return _is_prepared; } + { return _preparation; } /** - * Tells this we have done some operation where we should no longer consider ourself prepared + * \returns the \p Preparation structure with details about in what + * ways \p this mesh is currently prepared or unprepared. This + * structure may change in the future when cache designs change. + */ + struct Preparation; + + Preparation preparation () const + { return _preparation; } + + /** + * Tells this we have done some operation where we should no longer + * consider ourself prepared. This is a very coarse setting; it may + * be preferable to mark finer-grained settings instead. */ void set_isnt_prepared() - { _is_prepared = false; } + { _preparation = false; } + + /** + * Tells this we have done some operation creating unpartitioned + * elements. + */ + void set_isnt_partitioned() + { _preparation.is_partitioned = false; } + + /** + * Tells this we have done some operation (e.g. adding elements to a + * distributed mesh on one processor only) which can lose + * synchronization of id counts. + */ + void set_hasnt_synched_id_counts() + { _preparation.has_synched_id_counts = false; } + + /** + * Tells this we have done some operation (e.g. adding elements + * without setting their neighbor pointers) which requires neighbor + * pointers to be found later + */ + void set_hasnt_neighbor_ptrs() + { _preparation.has_neighbor_ptrs = false; } + + /** + * Tells this we have done some operation (e.g. adding elements with + * a new dimension or subdomain value) which may invalidate cached + * summaries of element data + */ + void set_hasnt_cached_elem_data() + { _preparation.has_cached_elem_data = false; } + + /** + * Tells this we have done some operation (e.g. refining elements + * with interior parents) which requires interior parent pointers to + * be found later + */ + void set_hasnt_interior_parent_ptrs() + { _preparation.has_interior_parent_ptrs = false; } + + /** + * Tells this we have done some operation (e.g. repartitioning) + * which may have left elements as ghosted which on a distributed + * mesh should be remote. + * + * User code should probably never need to use this; we can set it + * in Partitioner. + */ + void set_hasnt_removed_remote_elements() + { _preparation.has_removed_remote_elements = false; } + + /** + * Tells this we have done some operation (e.g. coarsening) + * which may have left orphaned nodes in need of removal. + * + * Most user code should probably never need to use this; we can set + * it in MeshRefinement. + */ + void set_hasnt_removed_orphaned_nodes() + { _preparation.has_removed_orphaned_nodes = false; } + + /** + * Tells this we have done some operation (e.g. adding or removing + * elements) which may require a reinit() of custom ghosting + * functors. + */ + void hasnt_reinit_ghosting_functors() + { _preparation.has_reinit_ghosting_functors = false; } + + /** + * Tells this we have done some operation (e.g. removing elements or + * adding new boundary entries) which may have invalidated our + * cached boundary id sets. + */ + void set_hasnt_boundary_id_sets() + { _preparation.has_boundary_id_sets = false; } /** * \returns \p true if all elements and nodes of the mesh @@ -260,7 +348,9 @@ class MeshBase : public ParallelObject * except for "ghosts" which touch a local element, and deletes * all nodes which are not part of a local or ghost element */ - virtual void delete_remote_elements () {} + virtual void delete_remote_elements () { + _preparation.has_removed_remote_elements = true; + } /** * Loops over ghosting functors and calls mesh_reinit() @@ -742,7 +832,7 @@ class MeshBase : public ParallelObject * To ensure a specific element id, call e->set_id() before adding it; * only do this in parallel if you are manually keeping ids consistent. * - * Users should call MeshBase::prepare_for_use() after elements are + * Users should call MeshBase::complete_preparation() after elements are * added to and/or deleted from the mesh. */ virtual Elem * add_elem (Elem * e) = 0; @@ -761,7 +851,7 @@ class MeshBase : public ParallelObject * Insert elem \p e to the element array, preserving its id * and replacing/deleting any existing element with the same id. * - * Users should call MeshBase::prepare_for_use() after elements are + * Users should call MeshBase::complete_preparation() after elements are * added to and/or deleted from the mesh. */ virtual Elem * insert_elem (Elem * e) = 0; @@ -780,8 +870,8 @@ class MeshBase : public ParallelObject * Removes element \p e from the mesh. This method must be * implemented in derived classes in such a way that it does not * invalidate element iterators. Users should call - * MeshBase::prepare_for_use() after elements are added to and/or - * deleted from the mesh. + * MeshBase::complete_preparation() after elements are added to + * and/or deleted from the mesh. * * \note Calling this method may produce isolated nodes, i.e. nodes * not connected to any element. @@ -848,7 +938,7 @@ class MeshBase : public ParallelObject /** * Removes any orphaned nodes, nodes not connected to any elements. - * Typically done automatically in prepare_for_use + * Typically done automatically in a preparation step */ void remove_orphaned_nodes (); @@ -1122,12 +1212,19 @@ class MeshBase : public ParallelObject const std::vector * default_values = nullptr); /** - * Prepare a newly ecreated (or read) mesh for use. - * This involves 4 steps: - * 1.) call \p find_neighbors() - * 2.) call \p partition() - * 3.) call \p renumber_nodes_and_elements() - * 4.) call \p cache_elem_data() + * Prepare a newly created (or read) mesh for use. + * This involves several steps: + * 1.) renumbering (if enabled) + * 2.) removing any orphaned nodes + * 3.) updating parallel id counts + * 4.) finding neighbor links + * 5.) caching summarized element data + * 6.) finding interior parent links + * 7.) clearing any old point locator + * 8.) calling reinit() on ghosting functors + * 9.) repartitioning (if enabled) + * 10.) removing any remote elements (if enabled) + * 11.) regenerating summarized boundary id sets * * The argument to skip renumbering is now deprecated - to prevent a * mesh from being renumbered, set allow_renumbering(false). The argument to skip @@ -1144,6 +1241,15 @@ class MeshBase : public ParallelObject #endif // LIBMESH_ENABLE_DEPRECATED void prepare_for_use (); + /* + * Prepare a newly created or modified mesh for use. + * + * Unlike \p prepare_for_use(), \p complete_preparation() performs + * *only* those preparatory steps that have been marked as + * necessary in the MeshBase::Preparation state. + */ + void complete_preparation(); + /** * Call the default partitioner (currently \p metis_partition()). */ @@ -1844,6 +1950,58 @@ class MeshBase : public ParallelObject const boundary_id_type b2); #endif + /** + * Flags indicating in what ways a mesh has been prepared for use. + */ + struct Preparation { + bool is_partitioned = false, + has_synched_id_counts = false, + has_neighbor_ptrs = false, + has_cached_elem_data = false, + has_interior_parent_ptrs = false, + has_removed_remote_elements = false, + has_removed_orphaned_nodes = false, + has_boundary_id_sets = false, + has_reinit_ghosting_functors = false; + + operator bool() const { + return is_partitioned && + has_synched_id_counts && + has_neighbor_ptrs && + has_cached_elem_data && + has_interior_parent_ptrs && + has_removed_remote_elements && + has_removed_orphaned_nodes && + has_reinit_ghosting_functors && + has_boundary_id_sets; + } + + Preparation & operator= (bool set_all) { + is_partitioned = set_all; + has_synched_id_counts = set_all; + has_neighbor_ptrs = set_all; + has_cached_elem_data = set_all; + has_interior_parent_ptrs = set_all; + has_removed_remote_elements = set_all; + has_removed_orphaned_nodes = set_all; + has_reinit_ghosting_functors = set_all; + has_boundary_id_sets = set_all; + + return *this; + } + + bool operator== (const Preparation & other) { + return is_partitioned == other.is_partitioned && + has_synched_id_counts == other.has_synched_id_counts && + has_neighbor_ptrs == other.has_neighbor_ptrs && + has_cached_elem_data == other.has_cached_elem_data && + has_interior_parent_ptrs == other.has_interior_parent_ptrs && + has_removed_remote_elements == other.has_removed_remote_elements && + has_removed_orphaned_nodes == other.has_removed_orphaned_nodes && + has_reinit_ghosting_functors == other.has_reinit_ghosting_functors && + has_boundary_id_sets == other.has_boundary_id_sets; + } + }; protected: @@ -1923,9 +2081,9 @@ class MeshBase : public ParallelObject unsigned char _default_mapping_data; /** - * Flag indicating if the mesh has been prepared for use. + * Flags indicating in what ways \p this mesh has been prepared. */ - bool _is_prepared; + Preparation _preparation; /** * A \p PointLocator class for this mesh. diff --git a/src/mesh/boundary_info.C b/src/mesh/boundary_info.C index b1878f510d..a50323ed7e 100644 --- a/src/mesh/boundary_info.C +++ b/src/mesh/boundary_info.C @@ -435,6 +435,8 @@ void BoundaryInfo::synchronize_global_id_set() libmesh_assert(_mesh); if (!_mesh->is_serial()) _communicator.set_union(_global_boundary_ids); + + _mesh->_preparation.has_boundary_id_sets = true; } diff --git a/src/mesh/distributed_mesh.C b/src/mesh/distributed_mesh.C index 24e352322d..5a15a04814 100644 --- a/src/mesh/distributed_mesh.C +++ b/src/mesh/distributed_mesh.C @@ -205,7 +205,7 @@ DistributedMesh::DistributedMesh (const MeshBase & other_mesh) : this->copy_constraint_rows(other_mesh); - this->_is_prepared = other_mesh.is_prepared(); + this->_preparation = other_mesh.preparation(); auto & this_boundary_info = this->get_boundary_info(); const auto & other_boundary_info = other_mesh.get_boundary_info(); @@ -291,6 +291,8 @@ void DistributedMesh::update_parallel_id_counts() ((_next_unique_id + this->n_processors() - 1) / (this->n_processors() + 1) + 1) * (this->n_processors() + 1) + this->processor_id(); #endif + + this->_preparation.has_synched_id_counts = true; } @@ -1611,6 +1613,8 @@ void DistributedMesh::renumber_nodes_and_elements () } } + this->_preparation.has_removed_orphaned_nodes = true; + if (_skip_renumber_nodes_and_elements) { this->update_parallel_id_counts(); @@ -1776,6 +1780,8 @@ void DistributedMesh::delete_remote_elements() this->libmesh_assert_valid_parallel_ids(); this->libmesh_assert_valid_parallel_flags(); #endif + + this->_preparation.has_removed_remote_elements = true; } diff --git a/src/mesh/mesh_base.C b/src/mesh/mesh_base.C index 535117af6b..28ee49c551 100644 --- a/src/mesh/mesh_base.C +++ b/src/mesh/mesh_base.C @@ -66,7 +66,7 @@ MeshBase::MeshBase (const Parallel::Communicator & comm_in, _n_parts (1), _default_mapping_type(LAGRANGE_MAP), _default_mapping_data(0), - _is_prepared (false), + _preparation (), _point_locator (), _count_lower_dim_elems_in_point_locator(true), _partitioner (), @@ -99,7 +99,7 @@ MeshBase::MeshBase (const MeshBase & other_mesh) : _n_parts (other_mesh._n_parts), _default_mapping_type(other_mesh._default_mapping_type), _default_mapping_data(other_mesh._default_mapping_data), - _is_prepared (other_mesh._is_prepared), + _preparation (other_mesh._preparation), _point_locator (), _count_lower_dim_elems_in_point_locator(other_mesh._count_lower_dim_elems_in_point_locator), _partitioner (), @@ -187,7 +187,7 @@ MeshBase& MeshBase::operator= (MeshBase && other_mesh) _n_parts = other_mesh.n_partitions(); _default_mapping_type = other_mesh.default_mapping_type(); _default_mapping_data = other_mesh.default_mapping_data(); - _is_prepared = other_mesh.is_prepared(); + _preparation = other_mesh._preparation; _point_locator = std::move(other_mesh._point_locator); _count_lower_dim_elems_in_point_locator = other_mesh.get_count_lower_dim_elems_in_point_locator(); #ifdef LIBMESH_ENABLE_UNIQUE_ID @@ -283,7 +283,7 @@ bool MeshBase::locally_equals (const MeshBase & other_mesh) const return false; if (_default_mapping_data != other_mesh._default_mapping_data) return false; - if (_is_prepared != other_mesh._is_prepared) + if (_preparation != other_mesh._preparation) return false; if (_count_lower_dim_elems_in_point_locator != other_mesh._count_lower_dim_elems_in_point_locator) @@ -794,6 +794,8 @@ void MeshBase::remove_orphaned_nodes () for (const auto & node : this->node_ptr_range()) if (!connected_nodes.count(node)) this->delete_node(node); + + _preparation.has_removed_orphaned_nodes = true; } @@ -834,9 +836,23 @@ void MeshBase::prepare_for_use (const bool skip_renumber_nodes_and_elements) + void MeshBase::prepare_for_use () { - LOG_SCOPE("prepare_for_use()", "MeshBase"); + // Mark everything as unprepared, except for those things we've been + // told we don't need to prepare, for backwards compatibility + this->clear_point_locator(); + _preparation = false; + _preparation.has_neighbor_ptrs = _skip_find_neighbors; + _preparation.has_removed_remote_elements = !_allow_remote_element_removal; + + this->complete_preparation(); +} + + +void MeshBase::complete_preparation() +{ + LOG_SCOPE("complete_preparation()", "MeshBase"); parallel_object_only(); @@ -871,15 +887,21 @@ void MeshBase::prepare_for_use () // using, but our partitioner might need that consistency and/or // might be confused by orphaned nodes. if (!_skip_renumber_nodes_and_elements) - this->renumber_nodes_and_elements(); + { + if (!_preparation.has_removed_orphaned_nodes || + !_preparation.has_synched_id_counts) + this->renumber_nodes_and_elements(); + } else { - this->remove_orphaned_nodes(); - this->update_parallel_id_counts(); + if (!_preparation.has_removed_orphaned_nodes) + this->remove_orphaned_nodes(); + if (!_preparation.has_synched_id_counts) + this->update_parallel_id_counts(); } // Let all the elements find their neighbors - if (!_skip_find_neighbors) + if (!_skip_find_neighbors && !_preparation.has_neighbor_ptrs) this->find_neighbors(); // The user may have set boundary conditions. We require that the @@ -892,11 +914,13 @@ void MeshBase::prepare_for_use () // Search the mesh for all the dimensions of the elements // and cache them. - this->cache_elem_data(); + if (!_preparation.has_cached_elem_data) + this->cache_elem_data(); // Search the mesh for elements that have a neighboring element // of dim+1 and set that element as the interior parent - this->detect_interior_parents(); + if (!_preparation.has_interior_parent_ptrs) + this->detect_interior_parents(); // Fix up node unique ids in case mesh generation code didn't take // exceptional care to do so. @@ -908,39 +932,41 @@ void MeshBase::prepare_for_use () MeshTools::libmesh_assert_valid_unique_ids(*this); #endif - // Reset our PointLocator. Any old locator is invalidated any time - // the elements in the underlying elements in the mesh have changed, - // so we clear it here. - this->clear_point_locator(); - // Allow our GhostingFunctor objects to reinit if necessary. // Do this before partitioning and redistributing, and before // deleting remote elements. - this->reinit_ghosting_functors(); + if (!_preparation.has_reinit_ghosting_functors) + this->reinit_ghosting_functors(); // Partition the mesh unless *all* partitioning is to be skipped. // If only noncritical partitioning is to be skipped, the // partition() call will still check for orphaned nodes. - if (!skip_partitioning()) + if (!skip_partitioning() && !_preparation.is_partitioned) this->partition(); + else + _preparation.is_partitioned = true; // If we're using DistributedMesh, we'll probably want it // parallelized. - if (this->_allow_remote_element_removal) + if (this->_allow_remote_element_removal && + !_preparation.has_removed_remote_elements) this->delete_remote_elements(); + else + _preparation.has_removed_remote_elements = true; // Much of our boundary info may have been for now-remote parts of the mesh, // in which case we don't want to keep local copies of data meant to be // local. On the other hand we may have deleted, or the user may have added in // a distributed fashion, boundary data that is meant to be global. So we // handle both of those scenarios here - this->get_boundary_info().regenerate_id_sets(); + if (!_preparation.has_boundary_id_sets) + this->get_boundary_info().regenerate_id_sets(); if (!_skip_renumber_nodes_and_elements) this->renumber_nodes_and_elements(); - // The mesh is now prepared for use. - _is_prepared = true; + // The mesh is now prepared for use, and it should know it. + libmesh_assert(_preparation); #ifdef DEBUG MeshTools::libmesh_assert_valid_boundary_ids(*this); @@ -958,6 +984,8 @@ MeshBase::reinit_ghosting_functors() libmesh_assert(gf); gf->mesh_reinit(); } + + _preparation.has_reinit_ghosting_functors = true; } void MeshBase::clear () @@ -965,8 +993,8 @@ void MeshBase::clear () // Reset the number of partitions _n_parts = 1; - // Reset the _is_prepared flag - _is_prepared = false; + // Reset the preparation flags + _preparation = false; // Clear boundary information if (boundary_info) @@ -1683,6 +1711,8 @@ void MeshBase::partition (const unsigned int n_parts) // Make sure any other locally cached data is correct this->update_post_partitioning(); } + + _preparation.is_partitioned = true; } void MeshBase::all_second_order (const bool full_ordered) @@ -1886,6 +1916,8 @@ void MeshBase::cache_elem_data() #endif } } + + _preparation.has_cached_elem_data = true; } @@ -1903,10 +1935,16 @@ void MeshBase::detect_interior_parents() // This requires an inspection on every processor parallel_object_only(); + // This requires up-to-date mesh dimensions in cache + libmesh_assert(_preparation.has_cached_elem_data); + // Check if the mesh contains mixed dimensions. If so, then we may // have interior parents to set. Otherwise return. if (this->elem_dimensions().size() == 1) - return; + { + _preparation.has_interior_parent_ptrs = true; + return; + } // Do we have interior parent pointers going to a different mesh? // If so then we'll still check to make sure that's the only place @@ -2002,6 +2040,8 @@ void MeshBase::detect_interior_parents() ("interior_parent() values in multiple meshes are unsupported."); } } + + _preparation.has_interior_parent_ptrs = true; } diff --git a/src/mesh/replicated_mesh.C b/src/mesh/replicated_mesh.C index 059d3fcbe7..b58d941156 100644 --- a/src/mesh/replicated_mesh.C +++ b/src/mesh/replicated_mesh.C @@ -115,7 +115,7 @@ ReplicatedMesh::ReplicatedMesh (const MeshBase & other_mesh) : this->copy_constraint_rows(other_mesh); - this->_is_prepared = other_mesh.is_prepared(); + this->_preparation = other_mesh.preparation(); auto & this_boundary_info = this->get_boundary_info(); const auto & other_boundary_info = other_mesh.get_boundary_info(); @@ -643,6 +643,8 @@ void ReplicatedMesh::update_parallel_id_counts() #ifdef LIBMESH_ENABLE_UNIQUE_ID _next_unique_id = this->parallel_max_unique_id(); #endif + + this->_preparation.has_synched_id_counts = true; } @@ -820,6 +822,8 @@ void ReplicatedMesh::renumber_nodes_and_elements () } } + this->_preparation.has_removed_orphaned_nodes = true; + libmesh_assert_equal_to (next_free_elem, _elements.size()); libmesh_assert_equal_to (next_free_node, _nodes.size()); diff --git a/src/mesh/unstructured_mesh.C b/src/mesh/unstructured_mesh.C index 935780b5dd..ac9da962c8 100644 --- a/src/mesh/unstructured_mesh.C +++ b/src/mesh/unstructured_mesh.C @@ -1298,15 +1298,15 @@ void UnstructuredMesh::find_neighbors (const bool reset_remote_elements, libmesh_assert(current_elem->interior_parent()); } } - #endif // AMR - #ifdef DEBUG MeshTools::libmesh_assert_valid_neighbors(*this, !reset_remote_elements); MeshTools::libmesh_assert_valid_amr_interior_parents(*this); #endif + + this->_preparation.has_neighbor_ptrs = true; } From 8e5fd887a52a3dea7a44c979ab1397f1411a540b Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Mon, 1 Dec 2025 11:51:52 -0600 Subject: [PATCH 02/15] Make PerfLog name accurate --- src/mesh/mesh_tools.C | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesh/mesh_tools.C b/src/mesh/mesh_tools.C index 7e98732dcb..7fdcf9406a 100644 --- a/src/mesh/mesh_tools.C +++ b/src/mesh/mesh_tools.C @@ -1227,7 +1227,7 @@ void clear_spline_nodes(MeshBase & mesh) bool valid_is_prepared (const MeshBase & mesh) { - LOG_SCOPE("libmesh_assert_valid_is_prepared()", "MeshTools"); + LOG_SCOPE("valid_is_prepared()", "MeshTools"); if (!mesh.is_prepared()) return true; From 809be07b955cce764768e05dac6480bb6a0aa334 Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Mon, 1 Dec 2025 14:40:57 -0600 Subject: [PATCH 03/15] More detailed preparation comments --- include/mesh/mesh_base.h | 57 ++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/include/mesh/mesh_base.h b/include/mesh/mesh_base.h index c30a559a49..8aaa3f944c 100644 --- a/include/mesh/mesh_base.h +++ b/include/mesh/mesh_base.h @@ -202,8 +202,9 @@ class MeshBase : public ParallelObject virtual void clear_elems () = 0; /** - * \returns \p true if the mesh has undergone all the preparation - * done in a call to \p prepare_for_use, \p false otherwise. + * \returns \p true if the mesh is marked as having undergone all of + * the preparation done in a call to \p prepare_for_use, \p false + * otherwise. */ bool is_prepared () const { return _preparation; } @@ -220,8 +221,8 @@ class MeshBase : public ParallelObject /** * Tells this we have done some operation where we should no longer - * consider ourself prepared. This is a very coarse setting; it may - * be preferable to mark finer-grained settings instead. + * consider ourself prepared. This is a very coarse setting; it is + * generally more efficient to mark finer-grained settings instead. */ void set_isnt_prepared() { _preparation = false; } @@ -229,22 +230,33 @@ class MeshBase : public ParallelObject /** * Tells this we have done some operation creating unpartitioned * elements. + * + * User code which adds elements to this mesh must either partition + * them too or call this method. */ void set_isnt_partitioned() { _preparation.is_partitioned = false; } /** - * Tells this we have done some operation (e.g. adding elements to a + * Tells this we have done some operation (e.g. adding objects to a * distributed mesh on one processor only) which can lose * synchronization of id counts. + * + * User code which does distributed additions of nodes or elements + * must call either this method or \p update_parallel_id_counts(). */ void set_hasnt_synched_id_counts() { _preparation.has_synched_id_counts = false; } /** * Tells this we have done some operation (e.g. adding elements - * without setting their neighbor pointers) which requires neighbor - * pointers to be found later + * without setting their neighbor pointers, or adding disjoint + * neighbor boundary pairs) which requires neighbor pointers to be + * determined later. + * + * User code which adds new elements to this mesh must call this + * function or manually set neighbor pointer from and to those + * elements. */ void set_hasnt_neighbor_ptrs() { _preparation.has_neighbor_ptrs = false; } @@ -252,7 +264,10 @@ class MeshBase : public ParallelObject /** * Tells this we have done some operation (e.g. adding elements with * a new dimension or subdomain value) which may invalidate cached - * summaries of element data + * summaries of element data. + * + * User code which adds new elements to this mesh must call this + * function. */ void set_hasnt_cached_elem_data() { _preparation.has_cached_elem_data = false; } @@ -260,7 +275,11 @@ class MeshBase : public ParallelObject /** * Tells this we have done some operation (e.g. refining elements * with interior parents) which requires interior parent pointers to - * be found later + * be found later. + * + * Most user code will not need to call this method; any user code + * that manipulates interior parents or their boundary elements may + * be an exception. */ void set_hasnt_interior_parent_ptrs() { _preparation.has_interior_parent_ptrs = false; } @@ -271,7 +290,10 @@ class MeshBase : public ParallelObject * mesh should be remote. * * User code should probably never need to use this; we can set it - * in Partitioner. + * in Partitioner. Any user code which manually repartitions + * elements on distributed meshes may need to call this manually, in + * addition to manually communicating elements with newly-created + * ghosting requirements. */ void set_hasnt_removed_remote_elements() { _preparation.has_removed_remote_elements = false; } @@ -281,7 +303,8 @@ class MeshBase : public ParallelObject * which may have left orphaned nodes in need of removal. * * Most user code should probably never need to use this; we can set - * it in MeshRefinement. + * it in MeshRefinement. User code which deletes elements without + * carefully deleting orphaned nodes should call this manually. */ void set_hasnt_removed_orphaned_nodes() { _preparation.has_removed_orphaned_nodes = false; } @@ -290,14 +313,20 @@ class MeshBase : public ParallelObject * Tells this we have done some operation (e.g. adding or removing * elements) which may require a reinit() of custom ghosting * functors. + * + * User code which adds or removes elements should call this method. + * User code which moves nodes ... should probably call this method, + * in case ghosting functors depending on position exist? */ void hasnt_reinit_ghosting_functors() { _preparation.has_reinit_ghosting_functors = false; } /** - * Tells this we have done some operation (e.g. removing elements or - * adding new boundary entries) which may have invalidated our - * cached boundary id sets. + * Tells this we have done some operation which may have invalidated + * our cached boundary id sets. + * + * User code which removes elements, or which adds or removes + * boundary entries, should call this method. */ void set_hasnt_boundary_id_sets() { _preparation.has_boundary_id_sets = false; } From 296bcd1f878b27468c45af1a9fb8b1e576608b89 Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Wed, 3 Dec 2025 13:13:42 -0600 Subject: [PATCH 04/15] Update comments --- include/mesh/mesh_base.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/include/mesh/mesh_base.h b/include/mesh/mesh_base.h index 8aaa3f944c..c373603445 100644 --- a/include/mesh/mesh_base.h +++ b/include/mesh/mesh_base.h @@ -519,16 +519,17 @@ class MeshBase : public ParallelObject * higher dimensions is checked. Also, x-z and y-z planar meshes are * considered to have spatial dimension == 3. * - * The spatial dimension is updated during prepare_for_use() based + * The spatial dimension is updated during mesh preparation based * on the dimensions of the various elements present in the Mesh, - * but is *never automatically decreased* by this function. + * but is *never automatically decreased*. * * For example, if the user calls set_spatial_dimension(2) and then * later inserts 3D elements into the mesh, * Mesh::spatial_dimension() will return 3 after the next call to - * prepare_for_use(). On the other hand, if the user calls - * set_spatial_dimension(3) and then inserts only x-aligned 1D - * elements into the Mesh, mesh.spatial_dimension() will remain 3. + * prepare_for_use() or complete_preparation(). On the other hand, + * if the user calls set_spatial_dimension(3) and then inserts only + * x-aligned 1D elements into the Mesh, mesh.spatial_dimension() + * will remain 3. */ unsigned int spatial_dimension () const; From 9386791e86c7f174bc77f055842c5f1c163d5a50 Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Wed, 3 Dec 2025 13:16:33 -0600 Subject: [PATCH 05/15] Prepare mesh_copy before smoother solves on it We need things like neighbor pointers, and in general we'd like to assert that we're never assembling or solving on unprepared meshes. --- src/mesh/mesh_smoother_vsmoother.C | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/mesh/mesh_smoother_vsmoother.C b/src/mesh/mesh_smoother_vsmoother.C index 5cbc842996..5efb135c9e 100644 --- a/src/mesh/mesh_smoother_vsmoother.C +++ b/src/mesh/mesh_smoother_vsmoother.C @@ -88,6 +88,16 @@ void VariationalMeshSmoother::setup() // Create a new mesh, EquationSystems, and System _mesh_copy = std::make_unique(_mesh); + + // If the _mesh wasn't prepared, that's fine (we'll just be moving + // its nodes), but we do need the copy to be prepared before our + // solve does things like looking at neighbors. We'll disable + // repartitioning and renumbering first to make sure that we can + // transfer our geometry changes back to the original mesh. + _mesh_copy->allow_renumbering(false); + _mesh_copy->skip_partitioning(true); + _mesh_copy->complete_preparation(); + _equation_systems = std::make_unique(*_mesh_copy); _system = &(_equation_systems->add_system("variational_smoother_system")); From b0a29d3fb0c9f0b6f182cbe5a3ecb3eb6bebf989 Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Wed, 3 Dec 2025 13:32:10 -0600 Subject: [PATCH 06/15] MeshModification shouldn't leave invalid caches We can rebuild point locators or element caches, but we don't want to use an already-built invalid cache by accident. --- src/mesh/mesh_modification.C | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/mesh/mesh_modification.C b/src/mesh/mesh_modification.C index a7429475e5..6015b81385 100644 --- a/src/mesh/mesh_modification.C +++ b/src/mesh/mesh_modification.C @@ -215,6 +215,10 @@ void MeshTools::Modification::distort (MeshBase & mesh, #endif } } + + // We haven't changed any topology, but just changing geometry could + // have invalidated a point locator. + mesh.clear_point_locator(); } @@ -302,6 +306,10 @@ void MeshTools::Modification::redistribute (MeshBase & mesh, (*node)(2) = output_vec(2); #endif } + + // We haven't changed any topology, but just changing geometry could + // have invalidated a point locator. + mesh.clear_point_locator(); } @@ -315,6 +323,10 @@ void MeshTools::Modification::translate (MeshBase & mesh, for (auto & node : mesh.node_ptr_range()) *node += p; + + // We haven't changed any topology, but just changing geometry could + // have invalidated a point locator. + mesh.clear_point_locator(); } @@ -346,6 +358,10 @@ MeshTools::Modification::rotate (MeshBase & mesh, const Real theta, const Real psi) { + // We won't change any topology, but just changing geometry could + // invalidate a point locator. + mesh.clear_point_locator(); + #if LIBMESH_DIM == 3 const auto R = RealTensorValue::intrinsic_rotation_matrix(phi, theta, psi); @@ -402,6 +418,10 @@ void MeshTools::Modification::scale (MeshBase & mesh, for (auto & node : mesh.node_ptr_range()) (*node)(2) *= z_scale; + + // We haven't changed any topology, but just changing geometry could + // have invalidated a point locator. + mesh.clear_point_locator(); } @@ -1425,8 +1445,6 @@ void MeshTools::Modification::all_tri (MeshBase & mesh) MeshCommunication().make_nodes_parallel_consistent (mesh); } - - // Prepare the newly created mesh for use. mesh.prepare_for_use(); @@ -1585,6 +1603,10 @@ void MeshTools::Modification::smooth (MeshBase & mesh, } } // refinement_level loop } // end iteration + + // We haven't changed any topology, but just changing geometry could + // have invalidated a point locator. + mesh.clear_point_locator(); } @@ -1741,6 +1763,10 @@ void MeshTools::Modification::change_subdomain_id (MeshBase & mesh, if (elem->subdomain_id() == old_id) elem->subdomain_id() = new_id; } + + // We just invalidated mesh.get_subdomain_ids(), but it might not be + // efficient to fix that here. + mesh.set_hasnt_cached_elem_data(); } From 14d637841f1735c046ba8de23852f548c1ccafc3 Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Thu, 4 Dec 2025 13:27:21 -0600 Subject: [PATCH 07/15] Clarify prepare_for_use() doxygen --- include/mesh/mesh_base.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/include/mesh/mesh_base.h b/include/mesh/mesh_base.h index c373603445..f0e659c640 100644 --- a/include/mesh/mesh_base.h +++ b/include/mesh/mesh_base.h @@ -1256,6 +1256,13 @@ class MeshBase : public ParallelObject * 10.) removing any remote elements (if enabled) * 11.) regenerating summarized boundary id sets * + * For backwards compatibility, prepare_for_use() performs *all* those + * steps, regardless of the official preparation() state of the + * mesh. In codes which have maintained a valid preparation() state + * via methods such as set_hasnt_synched_id_counts(), calling + * complete_preparation() will result in a fully-prepared mesh at + * less cost. + * * The argument to skip renumbering is now deprecated - to prevent a * mesh from being renumbered, set allow_renumbering(false). The argument to skip * finding neighbors is also deprecated. To prevent find_neighbors, set From b0367eccc7c25e025c4b79ea494aab2953299409 Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Mon, 15 Dec 2025 12:50:02 -0600 Subject: [PATCH 08/15] Handle skip_partitioning() in assertions If we're skipping partitioning, we shouldn't mark ourselves as partitioned if we're not, and we shouldn't expect that we're marked as partitioned. --- src/mesh/mesh_base.C | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/mesh/mesh_base.C b/src/mesh/mesh_base.C index 28ee49c551..5b68208387 100644 --- a/src/mesh/mesh_base.C +++ b/src/mesh/mesh_base.C @@ -943,7 +943,8 @@ void MeshBase::complete_preparation() // partition() call will still check for orphaned nodes. if (!skip_partitioning() && !_preparation.is_partitioned) this->partition(); - else + else if (!this->n_unpartitioned_elem() && + !this->n_unpartitioned_nodes()) _preparation.is_partitioned = true; // If we're using DistributedMesh, we'll probably want it @@ -965,8 +966,15 @@ void MeshBase::complete_preparation() if (!_skip_renumber_nodes_and_elements) this->renumber_nodes_and_elements(); - // The mesh is now prepared for use, and it should know it. - libmesh_assert(_preparation); + // The mesh is now prepared for use, with the possible exception of + // partitioning that was supposed to be skipped, and it should know + // it. +#ifndef NDEBUG + Preparation completed_preparation = _preparation; + if (skip_partitioning()) + completed_preparation.is_partitioned = true; + libmesh_assert(completed_preparation); +#endif #ifdef DEBUG MeshTools::libmesh_assert_valid_boundary_ids(*this); From d43cd7e2e2314becee3ab1cd170e11753d717bf4 Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Thu, 12 Feb 2026 09:42:45 -0700 Subject: [PATCH 09/15] Copy subdomains in copy constructor+assignment Not sure how we missed this for so long. --- src/mesh/mesh_base.C | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mesh/mesh_base.C b/src/mesh/mesh_base.C index 5b68208387..9282a104d3 100644 --- a/src/mesh/mesh_base.C +++ b/src/mesh/mesh_base.C @@ -119,6 +119,7 @@ MeshBase::MeshBase (const MeshBase & other_mesh) : _elem_dims(other_mesh._elem_dims), _elem_default_orders(other_mesh._elem_default_orders), _supported_nodal_order(other_mesh._supported_nodal_order), + _mesh_subdomains(other_mesh._mesh_subdomains), _elemset_codes_inverse_map(other_mesh._elemset_codes_inverse_map), _all_elemset_ids(other_mesh._all_elemset_ids), _spatial_dimension(other_mesh._spatial_dimension), @@ -207,6 +208,7 @@ MeshBase& MeshBase::operator= (MeshBase && other_mesh) _elem_dims = std::move(other_mesh.elem_dimensions()); _elem_default_orders = std::move(other_mesh.elem_default_orders()); _supported_nodal_order = other_mesh.supported_nodal_order(); + _mesh_subdomains = other_mesh._mesh_subdomains; _elemset_codes = std::move(other_mesh._elemset_codes); _elemset_codes_inverse_map = std::move(other_mesh._elemset_codes_inverse_map); _all_elemset_ids = std::move(other_mesh._all_elemset_ids); From 30adbe811b6c264d07593f15cc111e6eddbfa75e Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Thu, 12 Feb 2026 08:07:53 -0700 Subject: [PATCH 10/15] set_hasnt_ -> unset_has_ (/is) name changes --- include/mesh/mesh_base.h | 36 ++++++++++++++++++++++++++---------- src/mesh/mesh_modification.C | 2 +- src/mesh/unstructured_mesh.C | 2 +- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/include/mesh/mesh_base.h b/include/mesh/mesh_base.h index f0e659c640..3f83fe146b 100644 --- a/include/mesh/mesh_base.h +++ b/include/mesh/mesh_base.h @@ -219,12 +219,28 @@ class MeshBase : public ParallelObject Preparation preparation () const { return _preparation; } +#ifdef LIBMESH_ENABLE_DEPRECATED /** * Tells this we have done some operation where we should no longer * consider ourself prepared. This is a very coarse setting; it is * generally more efficient to mark finer-grained settings instead. + * + * This method name is now deprecated, in part to match the less + * awkward unset_has_ names of the more fine-grained methods, in + * part as a way to prompt older user codes to use the more + * fine-grained methods where they can, to speed up the + * complete_preparation() calls afterward. */ void set_isnt_prepared() + { libmesh_deprecated(); _preparation = false; } +#endif // LIBMESH_ENABLE_DEPRECATED + + /** + * Tells this we have done some operation where we should no longer + * consider ourself prepared. This is a very coarse setting; it is + * generally more efficient to mark finer-grained settings instead. + */ + void unset_is_prepared() { _preparation = false; } /** @@ -234,7 +250,7 @@ class MeshBase : public ParallelObject * User code which adds elements to this mesh must either partition * them too or call this method. */ - void set_isnt_partitioned() + void unset_is_partitioned() { _preparation.is_partitioned = false; } /** @@ -245,7 +261,7 @@ class MeshBase : public ParallelObject * User code which does distributed additions of nodes or elements * must call either this method or \p update_parallel_id_counts(). */ - void set_hasnt_synched_id_counts() + void unset_has_synched_id_counts() { _preparation.has_synched_id_counts = false; } /** @@ -258,7 +274,7 @@ class MeshBase : public ParallelObject * function or manually set neighbor pointer from and to those * elements. */ - void set_hasnt_neighbor_ptrs() + void unset_has_neighbor_ptrs() { _preparation.has_neighbor_ptrs = false; } /** @@ -269,7 +285,7 @@ class MeshBase : public ParallelObject * User code which adds new elements to this mesh must call this * function. */ - void set_hasnt_cached_elem_data() + void unset_has_cached_elem_data() { _preparation.has_cached_elem_data = false; } /** @@ -281,7 +297,7 @@ class MeshBase : public ParallelObject * that manipulates interior parents or their boundary elements may * be an exception. */ - void set_hasnt_interior_parent_ptrs() + void unset_has_interior_parent_ptrs() { _preparation.has_interior_parent_ptrs = false; } /** @@ -295,7 +311,7 @@ class MeshBase : public ParallelObject * addition to manually communicating elements with newly-created * ghosting requirements. */ - void set_hasnt_removed_remote_elements() + void unset_has_removed_remote_elements() { _preparation.has_removed_remote_elements = false; } /** @@ -306,7 +322,7 @@ class MeshBase : public ParallelObject * it in MeshRefinement. User code which deletes elements without * carefully deleting orphaned nodes should call this manually. */ - void set_hasnt_removed_orphaned_nodes() + void unset_has_removed_orphaned_nodes() { _preparation.has_removed_orphaned_nodes = false; } /** @@ -318,7 +334,7 @@ class MeshBase : public ParallelObject * User code which moves nodes ... should probably call this method, * in case ghosting functors depending on position exist? */ - void hasnt_reinit_ghosting_functors() + void unset_has_reinit_ghosting_functors() { _preparation.has_reinit_ghosting_functors = false; } /** @@ -328,7 +344,7 @@ class MeshBase : public ParallelObject * User code which removes elements, or which adds or removes * boundary entries, should call this method. */ - void set_hasnt_boundary_id_sets() + void unset_has_boundary_id_sets() { _preparation.has_boundary_id_sets = false; } /** @@ -1259,7 +1275,7 @@ class MeshBase : public ParallelObject * For backwards compatibility, prepare_for_use() performs *all* those * steps, regardless of the official preparation() state of the * mesh. In codes which have maintained a valid preparation() state - * via methods such as set_hasnt_synched_id_counts(), calling + * via methods such as unset_has_synched_id_counts(), calling * complete_preparation() will result in a fully-prepared mesh at * less cost. * diff --git a/src/mesh/mesh_modification.C b/src/mesh/mesh_modification.C index 6015b81385..4ee5f66911 100644 --- a/src/mesh/mesh_modification.C +++ b/src/mesh/mesh_modification.C @@ -1766,7 +1766,7 @@ void MeshTools::Modification::change_subdomain_id (MeshBase & mesh, // We just invalidated mesh.get_subdomain_ids(), but it might not be // efficient to fix that here. - mesh.set_hasnt_cached_elem_data(); + mesh.unset_has_cached_elem_data(); } diff --git a/src/mesh/unstructured_mesh.C b/src/mesh/unstructured_mesh.C index ac9da962c8..8602fba471 100644 --- a/src/mesh/unstructured_mesh.C +++ b/src/mesh/unstructured_mesh.C @@ -899,7 +899,7 @@ void UnstructuredMesh::copy_nodes_and_elements(const MeshBase & other_mesh, // actually be prepared now. if (skip_find_neighbors || !was_prepared || !other_mesh.is_prepared()) - this->set_isnt_prepared(); + this->unset_is_prepared(); } From 730d2c0cc7ef501be0135e482056007c595e8f36 Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Thu, 12 Feb 2026 08:08:50 -0700 Subject: [PATCH 11/15] Whitespace fix --- include/mesh/mesh_base.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/mesh/mesh_base.h b/include/mesh/mesh_base.h index 3f83fe146b..2a45f9ccb0 100644 --- a/include/mesh/mesh_base.h +++ b/include/mesh/mesh_base.h @@ -2006,7 +2006,8 @@ class MeshBase : public ParallelObject /** * Flags indicating in what ways a mesh has been prepared for use. */ - struct Preparation { + struct Preparation + { bool is_partitioned = false, has_synched_id_counts = false, has_neighbor_ptrs = false, From 63944e786d074bc6d3c5049758039a6e2021f34e Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Thu, 12 Feb 2026 09:27:00 -0700 Subject: [PATCH 12/15] copy_nodes_and_elements skip_preparation option My unit tests (in an upcoming commit) uncovered that it was impossible to copy a mesh with orphaned nodes that have boundary conditions, because copy_nodes_and_elements would helpfully delete them despite BoundaryInfo expecting to find them. But we do want to be able to copy even unprepared meshes without throwing an error. Instead of whining that it would be hard to break backwards compatibility here, I should have just added an option to drop backwards compatibility here. --- include/mesh/unstructured_mesh.h | 13 ++++- src/mesh/unstructured_mesh.C | 85 ++++++++++++++++++++------------ 2 files changed, 66 insertions(+), 32 deletions(-) diff --git a/include/mesh/unstructured_mesh.h b/include/mesh/unstructured_mesh.h index be3c5798f4..7c2bda2766 100644 --- a/include/mesh/unstructured_mesh.h +++ b/include/mesh/unstructured_mesh.h @@ -263,6 +263,16 @@ class UnstructuredMesh : public MeshBase * If an \p id_remapping map is provided, then element subdomain ids * in \p other_mesh will be converted using it before adding them to * \p this mesh. + * + * For backwards compatibility, this does some limited mesh + * preparation after the copy: everything except for renumbering, + * remote element removal, and partitioning. To skip just the + * step of that preparation which finds new neighbor_ptr links + * between elements, set \p skip_find_neighbors. To skip all of + * that preparation, set \p skip_preparation. If preparation is + * skipped, it is the users responsibility to set the flags + * indicating what preparation may still be necessary before using + * the mesh later. */ virtual void copy_nodes_and_elements (const MeshBase & other_mesh, const bool skip_find_neighbors = false, @@ -270,7 +280,8 @@ class UnstructuredMesh : public MeshBase dof_id_type node_id_offset = 0, unique_id_type unique_id_offset = 0, std::unordered_map * - id_remapping = nullptr); + id_remapping = nullptr, + const bool skip_preparation = false); /** * Move node and elements from other_mesh to this mesh. diff --git a/src/mesh/unstructured_mesh.C b/src/mesh/unstructured_mesh.C index 8602fba471..e2c48d9226 100644 --- a/src/mesh/unstructured_mesh.C +++ b/src/mesh/unstructured_mesh.C @@ -650,10 +650,15 @@ void UnstructuredMesh::copy_nodes_and_elements(const MeshBase & other_mesh, #endif , std::unordered_map * - id_remapping) + id_remapping, + const bool skip_preparation) { LOG_SCOPE("copy_nodes_and_elements()", "UnstructuredMesh"); + // If we're asked to skip all preparation, we should be skipping + // find_neighbors specifically. + libmesh_assert(!skip_preparation || skip_find_neighbors); + std::pair, std::vector> extra_int_maps = this->merge_extra_integer_names(other_mesh); @@ -867,39 +872,57 @@ void UnstructuredMesh::copy_nodes_and_elements(const MeshBase & other_mesh, this->set_next_unique_id(other_mesh.parallel_max_unique_id() + unique_id_offset + 1); #endif - // Finally, partially prepare the new Mesh for use. - // This is for backwards compatibility, so we don't want to prepare - // everything. - // - // Keep the same numbering and partitioning and distribution status - // for now, but save our original policies to restore later. - const bool allowed_renumbering = this->allow_renumbering(); - const bool allowed_find_neighbors = this->allow_find_neighbors(); - const bool allowed_elem_removal = this->allow_remote_element_removal(); - this->allow_renumbering(false); - this->allow_remote_element_removal(false); - this->allow_find_neighbors(!skip_find_neighbors); + // Finally, partially prepare the new Mesh for use, if that isn't + // being skipped. + // Even the default behavior here is for backwards compatibility, + // and we don't want to prepare everything. + + if (!skip_preparation) + { + // Keep the same numbering and partitioning and distribution + // status for now, but save our original policies to restore + // later. + const bool allowed_renumbering = this->allow_renumbering(); + const bool allowed_find_neighbors = this->allow_find_neighbors(); + const bool allowed_elem_removal = this->allow_remote_element_removal(); + this->allow_renumbering(false); + this->allow_remote_element_removal(false); + this->allow_find_neighbors(!skip_find_neighbors); - // We should generally be able to skip *all* partitioning here - // because we're only adding one already-consistent mesh to another. - const bool skipped_partitioning = this->skip_partitioning(); - this->skip_partitioning(true); + // We should generally be able to skip *all* partitioning here + // because we're only adding one already-consistent mesh to + // another. + const bool skipped_partitioning = this->skip_partitioning(); + this->skip_partitioning(true); - const bool was_prepared = this->is_prepared(); - this->prepare_for_use(); + const bool was_prepared = this->is_prepared(); + this->prepare_for_use(); + + //But in the long term, don't change our policies. + this->allow_find_neighbors(allowed_find_neighbors); + this->allow_renumbering(allowed_renumbering); + this->allow_remote_element_removal(allowed_elem_removal); + this->skip_partitioning(skipped_partitioning); + + // That prepare_for_use() call marked us as prepared, but we + // specifically avoided some important preparation, so we might not + // actually be prepared now. + if (skip_find_neighbors || + !was_prepared || !other_mesh.is_prepared()) + this->unset_is_prepared(); + } - //But in the long term, don't change our policies. - this->allow_find_neighbors(allowed_find_neighbors); - this->allow_renumbering(allowed_renumbering); - this->allow_remote_element_removal(allowed_elem_removal); - this->skip_partitioning(skipped_partitioning); - - // That prepare_for_use() call marked us as prepared, but we - // specifically avoided some important preparation, so we might not - // actually be prepared now. - if (skip_find_neighbors || - !was_prepared || !other_mesh.is_prepared()) - this->unset_is_prepared(); + // In general we've just invalidated just about everything, and we'd + // like to unset_is_prepared(), but specific use cases might know a + // priori that they're still partitioned well, or that they've + // copied in a disjoint mesh component and don't need new neighbor + // pointers, or that they're not adding anything that would change + // cached subdomain/element/boundary sets, etc., so we'll rely on + // users of the "advanced" skip_preparation option to also set what + // preparation they still need. + + // else + // this->unset_is_prepared(); } From ba88db9d7b849a6808fa74dccf33ff1904ac6c5a Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Thu, 12 Feb 2026 09:29:44 -0700 Subject: [PATCH 13/15] Use skip_preparation option in copy constructors --- src/mesh/distributed_mesh.C | 8 ++------ src/mesh/replicated_mesh.C | 8 ++------ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/mesh/distributed_mesh.C b/src/mesh/distributed_mesh.C index 5a15a04814..9fdc2e3fae 100644 --- a/src/mesh/distributed_mesh.C +++ b/src/mesh/distributed_mesh.C @@ -191,18 +191,14 @@ DistributedMesh::DistributedMesh (const MeshBase & other_mesh) : _next_free_unpartitioned_node_id(this->n_processors()), _next_free_unpartitioned_elem_id(this->n_processors()) { - this->copy_nodes_and_elements(other_mesh, true); + // Just copy, skipping preparation + this->copy_nodes_and_elements(other_mesh, true, 0, 0, 0, nullptr, true); this->allow_find_neighbors(other_mesh.allow_find_neighbors()); this->allow_renumbering(other_mesh.allow_renumbering()); this->allow_remote_element_removal(other_mesh.allow_remote_element_removal()); this->skip_partitioning(other_mesh.skip_partitioning()); - // The prepare_for_use() in copy_nodes_and_elements() is going to be - // tricky to remove without breaking backwards compatibility, but it - // updates some things we want to just copy. - this->copy_cached_data(other_mesh); - this->copy_constraint_rows(other_mesh); this->_preparation = other_mesh.preparation(); diff --git a/src/mesh/replicated_mesh.C b/src/mesh/replicated_mesh.C index b58d941156..2e8a4cf48f 100644 --- a/src/mesh/replicated_mesh.C +++ b/src/mesh/replicated_mesh.C @@ -101,18 +101,14 @@ ReplicatedMesh::ReplicatedMesh (const MeshBase & other_mesh) : UnstructuredMesh (other_mesh), _n_nodes(0), _n_elem(0) // copy_* will increment this { - this->copy_nodes_and_elements(other_mesh, true); + // Just copy, skipping preparation + this->copy_nodes_and_elements(other_mesh, true, 0, 0, 0, nullptr, true); this->allow_find_neighbors(other_mesh.allow_find_neighbors()); this->allow_renumbering(other_mesh.allow_renumbering()); this->allow_remote_element_removal(other_mesh.allow_remote_element_removal()); this->skip_partitioning(other_mesh.skip_partitioning()); - // The prepare_for_use() in copy_nodes_and_elements() is going to be - // tricky to remove without breaking backwards compatibility, but it - // updates some things we want to just copy. - this->copy_cached_data(other_mesh); - this->copy_constraint_rows(other_mesh); this->_preparation = other_mesh.preparation(); From a7258c03650e420853da15fd278c1f1a68d6a61f Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Thu, 12 Feb 2026 10:50:42 -0700 Subject: [PATCH 14/15] Weaken copy_nodes_and_elements assertion If we have a parallel-inconsistent partitioning then we may have a real problem, but we might temporarily have nodes that need to be repartitioned, and we'd like to be able to copy them anyway. --- src/mesh/unstructured_mesh.C | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mesh/unstructured_mesh.C b/src/mesh/unstructured_mesh.C index e2c48d9226..9463024124 100644 --- a/src/mesh/unstructured_mesh.C +++ b/src/mesh/unstructured_mesh.C @@ -677,10 +677,12 @@ void UnstructuredMesh::copy_nodes_and_elements(const MeshBase & other_mesh, // We're assuming the other mesh has proper element number ordering, // so that we add parents before their children, and that the other - // mesh is consistently partitioned. + // mesh is consistently partitioned. We're not assuming that node + // proc ids are topologically consistent, so we don't just + // libmesh_assert_valid_procids. #ifdef DEBUG MeshTools::libmesh_assert_valid_amr_elem_ids(other_mesh); - MeshTools::libmesh_assert_valid_procids(other_mesh); + MeshTools::libmesh_assert_parallel_consistent_procids(other_mesh); #endif //Copy in Nodes From a3571d70ab6b00235b6963352e90981ce40cc9f1 Mon Sep 17 00:00:00 2001 From: Roy Stogner Date: Thu, 12 Feb 2026 10:52:04 -0700 Subject: [PATCH 15/15] Add some unit tests for partial mesh preparation This hits the easiest-to-set-up partially-unprepared cases, and even that was enough to expose a subtle bug or two. --- tests/mesh/mesh_base_test.C | 145 ++++++++++++++++++++++++++++++++++-- 1 file changed, 140 insertions(+), 5 deletions(-) diff --git a/tests/mesh/mesh_base_test.C b/tests/mesh/mesh_base_test.C index 3c4902017a..f97cb1cea4 100644 --- a/tests/mesh/mesh_base_test.C +++ b/tests/mesh/mesh_base_test.C @@ -22,6 +22,15 @@ public: /* Tests need a 2d mesh */ #if LIBMESH_DIM > 1 + CPPUNIT_TEST( testDistributedMeshVerifyHasNeighborPtrs ); + CPPUNIT_TEST( testMeshVerifyHasNeighborPtrs ); + CPPUNIT_TEST( testReplicatedMeshVerifyHasNeighborPtrs ); + CPPUNIT_TEST( testDistributedMeshVerifyHasCachedElemData ); + CPPUNIT_TEST( testMeshVerifyHasCachedElemData ); + CPPUNIT_TEST( testReplicatedMeshVerifyHasCachedElemData ); + CPPUNIT_TEST( testDistributedMeshVerifyRemovalPreparation ); + CPPUNIT_TEST( testMeshVerifyRemovalPreparation ); + CPPUNIT_TEST( testReplicatedMeshVerifyRemovalPreparation ); CPPUNIT_TEST( testDistributedMeshVerifyIsPrepared ); CPPUNIT_TEST( testMeshVerifyIsPrepared ); CPPUNIT_TEST( testReplicatedMeshVerifyIsPrepared ); @@ -35,7 +44,7 @@ public: void tearDown() {} - void testMeshBaseVerifyIsPrepared(UnstructuredMesh & mesh) + void BrokenNeighborMesh(UnstructuredMesh & mesh) { /** * Build a 2d 2x2 square mesh (mesh_one) covering [0.0, 1.0] x [0.0, 1.0] @@ -47,10 +56,6 @@ public: 0., 1., QUAD9); - // build_square does its own prepare_for_use(), so we should be - // prepared. - CPPUNIT_ASSERT(MeshTools::valid_is_prepared(mesh)); - // Break some neighbor links. Of course nobody would do this in // real life, right? Elem * elem0 = mesh.query_elem_ptr(0); @@ -66,6 +71,136 @@ public: elem0->set_neighbor(n, nullptr); neigh->set_neighbor(n_neigh, nullptr); } + } + + void testMeshBaseVerifyHasNeighborPtrs(UnstructuredMesh & mesh) + { + this->BrokenNeighborMesh(mesh); + mesh.unset_has_neighbor_ptrs(); + mesh.complete_preparation(); + CPPUNIT_ASSERT(mesh.is_prepared()); + CPPUNIT_ASSERT(MeshTools::valid_is_prepared(mesh)); + } + + void testDistributedMeshVerifyHasNeighborPtrs () + { + DistributedMesh mesh(*TestCommWorld); + testMeshBaseVerifyHasNeighborPtrs(mesh); + } + + void testMeshVerifyHasNeighborPtrs () + { + Mesh mesh(*TestCommWorld); + testMeshBaseVerifyHasNeighborPtrs(mesh); + } + + void testReplicatedMeshVerifyHasNeighborPtrs () + { + ReplicatedMesh mesh(*TestCommWorld); + testMeshBaseVerifyHasNeighborPtrs(mesh); + } + + void testMeshBaseVerifyHasCachedElemData(UnstructuredMesh & mesh) + { + /** + * Build a 2d 2x2 square mesh (mesh_one) covering [0.0, 1.0] x [0.0, 1.0] + * with linear Quad elements. + */ + MeshTools::Generation::build_square(mesh, + 2, 2, + 0., 1., + 0., 1., + QUAD9); + + // Invalidate the subdomain ids cache + Elem * elem0 = mesh.query_elem_ptr(0); + if (elem0) + elem0->subdomain_id() = 1; + + // We're unprepared (prepare_for_use() will update that cache) but + // we're not marked that way. + CPPUNIT_ASSERT(!MeshTools::valid_is_prepared(mesh)); + + mesh.unset_has_cached_elem_data(); + mesh.complete_preparation(); + CPPUNIT_ASSERT(mesh.is_prepared()); + CPPUNIT_ASSERT(MeshTools::valid_is_prepared(mesh)); + } + + void testDistributedMeshVerifyHasCachedElemData () + { + DistributedMesh mesh(*TestCommWorld); + testMeshBaseVerifyHasCachedElemData(mesh); + } + + void testMeshVerifyHasCachedElemData () + { + Mesh mesh(*TestCommWorld); + testMeshBaseVerifyHasCachedElemData(mesh); + } + + void testReplicatedMeshVerifyHasCachedElemData () + { + ReplicatedMesh mesh(*TestCommWorld); + testMeshBaseVerifyHasCachedElemData(mesh); + } + + void testMeshBaseVerifyRemovalPreparation(UnstructuredMesh & mesh) + { + /** + * Build a 2d 2x2 square mesh (mesh_one) covering [0.0, 1.0] x [0.0, 1.0] + * with linear Quad elements. + */ + MeshTools::Generation::build_square(mesh, + 2, 2, + 0., 1., + 0., 1., + QUAD9); + + // Remove elements on one side, orphaning 4 nodes and removing one + // boundary condition. Remove dangling neighbor pointers too; we + // can't even clone a mesh with dangling pointers. + for (auto & elem : mesh.element_ptr_range()) + if (elem->vertex_average()(0) > 0.5) + mesh.delete_elem(elem); + else + elem->set_neighbor(1, nullptr); + + // We're unprepared (prepare_for_use() will remove those orphaned + // nodes and fix the boundary id sets and fix the partitioning of + // nodes that might need new owners) but we're not marked that + // way. + CPPUNIT_ASSERT(!MeshTools::valid_is_prepared(mesh)); + + mesh.unset_is_partitioned(); + mesh.unset_has_removed_orphaned_nodes(); + mesh.unset_has_boundary_id_sets(); + mesh.complete_preparation(); + CPPUNIT_ASSERT(mesh.is_prepared()); + CPPUNIT_ASSERT(MeshTools::valid_is_prepared(mesh)); + } + + void testDistributedMeshVerifyRemovalPreparation () + { + DistributedMesh mesh(*TestCommWorld); + testMeshBaseVerifyRemovalPreparation(mesh); + } + + void testMeshVerifyRemovalPreparation () + { + Mesh mesh(*TestCommWorld); + testMeshBaseVerifyRemovalPreparation(mesh); + } + + void testReplicatedMeshVerifyRemovalPreparation () + { + ReplicatedMesh mesh(*TestCommWorld); + testMeshBaseVerifyRemovalPreparation(mesh); + } + + void testMeshBaseVerifyIsPrepared(UnstructuredMesh & mesh) + { + this->BrokenNeighborMesh(mesh); // We're unprepared (prepare_for_use() will restitch those // neighbor pointers) but we're not marked that way.