Mesh preparation assertions

include/mesh/distributed_mesh.h

@@ -111,7 +111,7 @@ class DistributedMesh : public UnstructuredMesh
    * Shim to allow operator == (&) to behave like a virtual function
    * without having to be one.
    */
-  virtual bool subclass_locally_equals (const MeshBase & other_mesh) const override;
+  virtual std::string_view subclass_first_difference_from (const MeshBase & other_mesh_base) const override;
 
   /**
    * Virtual copy-constructor, creates a copy of this mesh

include/mesh/mesh_base.h

@@ -130,7 +130,7 @@ class MeshBase : public ParallelObject
    * ids).
    *
    * Though this method is non-virtual, its implementation calls the
-   * virtual function \p subclass_locally_equals() to test for
+   * virtual function \p subclass_first_difference_from() to test for
    * equality of subclass-specific data as well.
    */
   bool operator== (const MeshBase & other_mesh) const;
@@ -140,6 +140,14 @@ class MeshBase : public ParallelObject
     return !(*this == other_mesh);
   }
 
+  /**
+   * This behaves like libmesh_assert(*this == other_mesh), but gives
+   * a more useful accounting of the first difference found, if the
+   * assertion fails.
+   */
+  void assert_equal_to (const MeshBase & other_mesh,
+                        std::string_view failure_context) const;
+
   /**
    * This behaves the same as operator==, but only for the local and
    * ghosted aspects of the mesh; i.e. operator== is true iff local
@@ -427,23 +435,20 @@ class MeshBase : public ParallelObject
    * \returns A const reference to a std::set of element dimensions
    * present in the mesh.
    */
-  const std::set<unsigned char> & elem_dimensions() const
-  { return _elem_dims; }
+  const std::set<unsigned char> & elem_dimensions() const;
 
   /**
    * \returns A const reference to a std::set of element default
    * orders present in the mesh.
    */
-  const std::set<Order> & elem_default_orders() const
-  { return _elem_default_orders; }
+  const std::set<Order> & elem_default_orders() const;
 
   /**
    * \returns The smallest supported_nodal_order() of any element
    * present in the mesh, which is thus the maximum supported nodal
    * order on the mesh as a whole.
    */
-  Order supported_nodal_order() const
-  { return _supported_nodal_order; }
+  Order supported_nodal_order() const;
 
   /**
    * Most of the time you should not need to call this, as the element
@@ -986,10 +991,17 @@ class MeshBase : public ParallelObject
    *
    * If \p assert_valid is left as true, then in dbg mode extensive
    * consistency checking is performed before returning.
+   *
+   * If \p check_non_remote is set to false, then only sides which
+   * currently have remote neighbors are checked for possible local
+   * neighbors.  This is intended to handle a corner case where
+   * ancestor neighbors are redistributed to a processor only by other
+   * processors who do not see that neighbor link.
    */
   virtual void find_neighbors (const bool reset_remote_elements = false,
                                const bool reset_current_list    = true,
-                               const bool assert_valid          = true) = 0;
+                               const bool assert_valid          = true,
+                               const bool check_non_remote      = true) = 0;
 
   /**
    * Removes any orphaned nodes, nodes not connected to any elements.
@@ -2054,8 +2066,7 @@ class MeshBase : public ParallelObject
    * should contain all the subdomain ids across processors. Relies on the mesh
    * being prepared
    */
-  const std::set<subdomain_id_type> & get_mesh_subdomains() const
-  { libmesh_assert(this->is_prepared()); return _mesh_subdomains; }
+  const std::set<subdomain_id_type> & get_mesh_subdomains() const;
 
 #ifdef LIBMESH_ENABLE_PERIODIC
   /**
@@ -2100,6 +2111,9 @@ class MeshBase : public ParallelObject
     bool operator== (const Preparation & other) const;
     bool operator!= (const Preparation & other) const;
 
+    // Assert that a Preparation object is identical across processors
+    void libmesh_assert_consistent (const Parallel::Communicator & libmesh_dbg_var(comm));
+
     bool is_partitioned;
     bool has_synched_id_counts;
     bool has_neighbor_ptrs;
@@ -2150,14 +2164,20 @@ class MeshBase : public ParallelObject
    * Shim to allow operator == (&) to behave like a virtual function
    * without having to be one.
    */
-  virtual bool subclass_locally_equals (const MeshBase & other_mesh) const = 0;
+  virtual std::string_view subclass_first_difference_from (const MeshBase & other_mesh) const = 0;
 
   /**
    * Tests for equality of all elements and nodes in the mesh.  Helper
    * function for subclass_equals() in unstructured mesh subclasses.
    */
   bool nodes_and_elements_equal(const MeshBase & other_mesh) const;
 
+  /**
+   *
+   */
+  std::string_view first_difference_from(const MeshBase & other_mesh) const;
+
+
   /**
    * \returns A writable reference to the number of partitions.
    */
@@ -2625,6 +2645,48 @@ MeshBase::const_node_iterator : MeshBase::const_node_filter_iter
 };
 
 
+// ------------------------------------------------------------
+// Elem class member functions
+inline
+const std::set<unsigned char> & MeshBase::elem_dimensions() const
+{
+  libmesh_assert(_preparation.has_cached_elem_data);
+  return _elem_dims;
+}
+
+
+inline
+const std::set<Order> & MeshBase::elem_default_orders() const
+{
+  libmesh_assert(_preparation.has_cached_elem_data);
+  return _elem_default_orders;
+}
+
+
+inline
+Order MeshBase::supported_nodal_order() const
+{
+  libmesh_assert(_preparation.has_cached_elem_data);
+  return _supported_nodal_order;
+}
+
+
+inline
+const std::set<subdomain_id_type> & MeshBase::get_mesh_subdomains() const
+{
+  libmesh_assert(_preparation.has_cached_elem_data);
+  return _mesh_subdomains;
+}
+
+
+inline
+unsigned int MeshBase::spatial_dimension () const
+{
+  libmesh_assert(_preparation.has_cached_elem_data);
+
+  return cast_int<unsigned int>(_spatial_dimension);
+}
+
 template <typename T>
 inline
 unsigned int MeshBase::add_elem_datum(const std::string & name,

include/mesh/mesh_tools.h

@@ -394,24 +394,35 @@ void clear_spline_nodes(MeshBase &);
 
 /**
  * A function for testing whether a mesh's cached is_prepared() setting
- * is not a false positive.  If the mesh is marked as not prepared, or
- * if preparing the already-partitioned mesh (without any
- * repartitioning or renumbering) does not change it, then we return
- * true.  If the mesh believes it is prepared but prepare_for_use()
- * would change it, we return false.
+ * is not a false positive, and that its preparation() values are not
+ * false positives.  If the mesh is marked as completely not prepared, or
+ * if preparing a already-prepared mesh (without any repartitioning or
+ * renumbering) or preparing after a complete_preparation() (likewise)
+ * does not change the mesh, then we return true.  If the mesh believes it
+ * is prepared but prepare_for_use() would change it, or if the mesh
+ * believes it is partially prepared in a certain way but
+ * complete_preparation() does not completely prepare it, we return
+ * false.
  */
 bool valid_is_prepared (const MeshBase & mesh);
 
 ///@{
 
 /**
- * The following functions, only available in builds with NDEBUG
+ * The following functions, no-ops except in builds with NDEBUG
  * undefined, are for asserting internal consistency that we hope
  * should never be broken in opt
  */
 
 #ifndef NDEBUG
 
+/**
+ * Like libmesh_assert(MeshTools::valid_is_prepared(mesh)), but
+ * provides more information on preparation incompleteness in case of
+ * an error.
+ */
+void libmesh_assert_valid_is_prepared (const MeshBase & mesh);
+
 /**
  * A function for testing that all DofObjects within a mesh
  * have the same n_systems count
@@ -644,6 +655,39 @@ namespace Private {
 void globally_renumber_nodes_and_elements (MeshBase &);
 } // end namespace Private
 
+
+// Declare inline no-ops for assertions with NDEBUG defined
+#ifdef NDEBUG
+
+inline void libmesh_assert_valid_is_prepared (const MeshBase &) {}
+
+inline void libmesh_assert_equal_n_systems (const MeshBase &) {}
+
+inline void libmesh_assert_old_dof_objects (const MeshBase &) {}
+
+inline void libmesh_assert_valid_node_pointers (const MeshBase &) {}
+
+inline void libmesh_assert_valid_remote_elems (const MeshBase &) {}
+
+inline void libmesh_assert_valid_elem_ids (const MeshBase &) {}
+
+inline void libmesh_assert_valid_amr_elem_ids (const MeshBase &) {}
+
+inline void libmesh_assert_valid_amr_interior_parents (const MeshBase &) {}
+
+inline void libmesh_assert_contiguous_dof_ids (const MeshBase &, unsigned int) {}
+
+template <typename DofObjectSubclass>
+inline void libmesh_assert_topology_consistent_procids (const MeshBase &) {}
+
+inline void libmesh_assert_canonical_node_procids (const MeshBase &) {}
+
+inline void libmesh_assert_valid_refinement_tree (const MeshBase &) {}
+
+#endif // NDEBUG
+
+
+
 } // end namespace MeshTools
 
 } // namespace libMesh

include/mesh/replicated_mesh.h

@@ -97,7 +97,7 @@ class ReplicatedMesh : public UnstructuredMesh
    * Shim to allow operator == (&) to behave like a virtual function
    * without having to be one.
    */
-  virtual bool subclass_locally_equals (const MeshBase & other_mesh) const override;
+  virtual std::string_view subclass_first_difference_from (const MeshBase & other_mesh_base) const override;
 
   /**
    * Virtual copy-constructor, creates a copy of this mesh

include/mesh/unstructured_mesh.h

@@ -295,7 +295,8 @@ class UnstructuredMesh : public MeshBase
    */
   virtual void find_neighbors (const bool reset_remote_elements = false,
                                const bool reset_current_list    = true,
-                               const bool assert_valid          = true) override;
+                               const bool assert_valid          = true,
+                               const bool check_non_remote      = true) override;
 
 #ifdef LIBMESH_ENABLE_AMR
   /**

src/mesh/distributed_mesh.C

@@ -95,48 +95,52 @@ MeshBase & DistributedMesh::assign(MeshBase && other_mesh)
   return *this;
 }
 
-bool DistributedMesh::subclass_locally_equals(const MeshBase & other_mesh_base) const
+std::string_view DistributedMesh::subclass_first_difference_from (const MeshBase & other_mesh_base) const
 {
   const DistributedMesh * dist_mesh_ptr =
     dynamic_cast<const DistributedMesh *>(&other_mesh_base);
   if (!dist_mesh_ptr)
-    return false;
+    return "DistributedMesh class";
   const DistributedMesh & other_mesh = *dist_mesh_ptr;
 
-  if (_is_serial != other_mesh._is_serial ||
-      _is_serial_on_proc_0 != other_mesh._is_serial_on_proc_0 ||
-      _deleted_coarse_elements != other_mesh._deleted_coarse_elements ||
-      _n_nodes != other_mesh._n_nodes ||
-      _n_elem != other_mesh._n_elem ||
-      _max_node_id != other_mesh._max_node_id ||
-      _max_elem_id != other_mesh._max_elem_id ||
-      // We expect these things to change in a prepare_for_use();
-      // they're conceptually "mutable"...
+#define CHECK_MEMBER(member_name) \
+  if (member_name != other_mesh.member_name) \
+    return #member_name;
+
+  CHECK_MEMBER(_is_serial);
+  CHECK_MEMBER(_is_serial_on_proc_0);
+  CHECK_MEMBER(_deleted_coarse_elements);
+  CHECK_MEMBER(_n_nodes);
+  CHECK_MEMBER(_n_elem);
+  CHECK_MEMBER(_max_node_id);
+  CHECK_MEMBER(_max_elem_id);
+  // We expect these things to change in a prepare_for_use();
+  // they're conceptually "mutable"...
 /*
-      _next_free_local_node_id != other_mesh._next_free_local_node_id ||
-      _next_free_local_elem_id != other_mesh._next_free_local_elem_id ||
-      _next_free_unpartitioned_node_id != other_mesh._next_free_unpartitioned_node_id ||
-      _next_free_unpartitioned_elem_id != other_mesh._next_free_unpartitioned_elem_id ||
+  CHECK_MEMBER(_next_free_local_node_id);
+  CHECK_MEMBER(_next_free_local_elem_id);
+  CHECK_MEMBER(_next_free_unpartitioned_node_id);
+  CHECK_MEMBER(_next_free_unpartitioned_elem_id);
 #ifdef LIBMESH_ENABLE_UNIQUE_ID
-      _next_unpartitioned_unique_id != other_mesh._next_unpartitioned_unique_id ||
+  CHECK_MEMBER(_next_unpartitioned_unique_id);
 #endif
 */
-      !this->nodes_and_elements_equal(other_mesh))
-    return false;
+  if (!this->nodes_and_elements_equal(other_mesh))
+    return "nodes and/or elements";
 
   if (_extra_ghost_elems.size() !=
       other_mesh._extra_ghost_elems.size())
-    return false;
+    return "_extra_ghost_elems size";
   for (auto & elem : _extra_ghost_elems)
     {
       libmesh_assert(this->query_elem_ptr(elem->id()) == elem);
       const Elem * other_elem = other_mesh.query_elem_ptr(elem->id());
       if (!other_elem ||
           !other_mesh._extra_ghost_elems.count(const_cast<Elem *>(other_elem)))
-        return false;
+        return "_extra_ghost_elems entry";
     }
 
-  return true;
+  return "";
 }
 
 DistributedMesh::~DistributedMesh ()
@@ -1055,9 +1059,13 @@ void DistributedMesh::redistribute ()
 
       this->update_parallel_id_counts();
 
-      // We ought to still have valid neighbor links; we communicate
-      // them for newly-redistributed elements
-      // this->find_neighbors();
+      // We communicate valid neighbor links for newly-redistributed
+      // elements, but we may still have remote_elem links that should
+      // be corrected but whose correction wasn't communicated.
+      this->find_neighbors(/*reset_remote_elements*/ false,
+                           /*reset_current_list*/ false /*non-default*/,
+                           /*assert_valid*/ false,
+                           /*check_non_remote*/ false /*non-default!*/);
 
       // Is this necessary?  If we are called from prepare_for_use(), this will be called
       // anyway... but users can always call partition directly, in which case we do need