idaholab / moose

Multiphysics Object Oriented Simulation Environment
https://www.mooseframework.org
GNU Lesser General Public License v2.1
1.7k stars 1.04k forks source link

Not preparing for use before exodus output can leave the mesh incompatible with Paraview #27668

Open GiudGiud opened 3 months ago

GiudGiud commented 3 months ago

Bug Description

I was modifying the AdvancedExtruderGenerator to handle Quad8 and it left a lot of issues in the boundary_info that could have been handled by a prepare_for_use before outputting the mesh

the problem is that in the process of mesh generator, more nodes are added to the mesh and the boundary_info than necessary. I could see element deletion generators facing the same issues. If we delete elements but leave their nodes in the boundary info

note that this would be an issue also for intermediate post-each mesh generator outputs

Paraview crashes on having nodes in nodesets that are not present in the mesh Exodiff reports cryptic diffs with seemingly some OOB indexes

Steps to Reproduce

Use the AdvancedExtruded with QUAD8, dont prepare for use

Impact

Not much now that we are aware of the problem Preparing for use could be potentially expensive which is why we dont do it

[Optional] Diagnostics

No response

lindsayad commented 3 months ago

When are you outputting the mesh? We should be always preparing for use after the final mesh generator has run

GiudGiud commented 3 months ago

Even in mesh-only mode? I m looking at the mesh in mesh-only mode after the extrusion generator has ran on Quad8s

lindsayad commented 3 months ago

Yes, this code should still get called

bool
MooseMesh::prepare(const MeshBase * const mesh_to_clone)
{
  TIME_SECTION("prepare", 2, "Preparing Mesh", true);

  bool called_prepare_for_use = false;

  mooseAssert(_mesh, "The MeshBase has not been constructed");

  if (!dynamic_cast<DistributedMesh *>(&getMesh()) || _is_nemesis)
    // For whatever reason we do not want to allow renumbering here nor ever in the future?
    getMesh().allow_renumbering(false);

  if (mesh_to_clone)
  {
    mooseAssert(mesh_to_clone->is_prepared(),
                "The mesh we wish to clone from must already be prepared");
    _mesh = mesh_to_clone->clone();
    _moose_mesh_prepared = false;
  }
  else if (!_mesh->is_prepared())
  {
    _mesh->prepare_for_use();
    _moose_mesh_prepared = false;
    called_prepare_for_use = true;
  }

  if (_moose_mesh_prepared)
    return called_prepare_for_use;
GiudGiud commented 3 months ago

Weird. Adding a prepare_for_use right after extrusion fixed my problem for sure. and the mesh was definitely marked as not prepared at the end of that.

GiudGiud commented 3 months ago

looking at #25345 this should have already been addressed.