Shared mesh in StateManager

[white238]

Fixes #1443

• Adds a shared_ptr overload to setMesh()
• Converts the given unique_ptr mesh to a shared_ptr
• Adds two new lists to handle the memory lifecycle of duals and states created by the StateManager
• Clarified a lot of comments about ownership since StateManager handles most of our unique memory ownership issues like restart/not, shared/owned mesh

[white238]

@SchmidtMat

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes #1443 by ensuring meshes registered with StateManager can be shared externally without being deleted on reset(), and by tightening lifetime management for registered MFEM fields.

Changes:

• Add StateManager::setMesh(std::shared_ptr<...>) and internally retain shared ownership of meshes via a meshes_ map.
• Track ParGridFunction lifetimes created during storeState() / storeDual() using owned_states_ / owned_duals_ instead of leaking raw new allocations.
• Update documentation and add a regression test ensuring reset() doesn’t delete an externally-shared mesh.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/smith/physics/state/tests/test_state_manager.cpp	Adds regression test for shared mesh lifetime across StateManager::reset().
src/smith/physics/state/state_manager.hpp	Introduces shared-mesh API, mesh ownership map, and owned grid-function containers; updates docs/comments.
src/smith/physics/state/state_manager.cpp	Implements shared/unique setMesh behavior and owned grid-function tracking in storeState/storeDual.
src/smith/physics/mesh.hpp	Clarifies Mesh stores a non-owning mfem::ParMesh*.
src/smith/physics/mesh.cpp	Adds <memory> include (likely supporting shared ownership usage).
src/smith/numerics/tests/test_trust_region_solver.cpp	Adds <memory> include.
src/docs/sphinx/dev_guide/state_manager.rst	Updates developer guide to reflect new setMesh() ownership semantics and restart ownership model.

Comments suppressed due to low confidence (1)

src/smith/physics/state/state_manager.hpp:352

• In reset(), owned_states_ / owned_duals_ are cleared before datacolls_. Since the data collections still hold raw pointers to registered mfem::ParGridFunction objects, this creates dangling pointers inside datacolls_ until it is cleared (and can become a use-after-free if MFEMSidreDataCollection performs any cleanup involving those pointers). Reorder teardown so datacolls_.clear() happens before deleting the owned grid functions (i.e., clear owned_states_ / owned_duals_ after datacolls_).

  {
    named_states_.clear();
    named_duals_.clear();
    owned_states_.clear();
    owned_duals_.clear();
    shape_displacements_.clear();
    shape_displacement_duals_.clear();
    datacolls_.clear();
    meshes_.clear();
    output_dir_.clear();
    is_restart_ = false;
    ds_ = nullptr;
  };

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[tupek2]

I think the only real 'debate' we could consider. Does it make sense to handle this complexity of having the Sidre own the mesh only when its a restart. Or is it somehow easier to just require that on restart the user is requires to point to the same mesh again. If the extra code is minimal to handle this complexity of changing what owns the mesh depending on restart vs. an initial simulation, then I think this is OK. If the code is a lot simpler to just say that the mesh is ALWAYs a shared_ptr, then I'd be happy to force users to just point out the mesh file again. No real strong opinion, its just what came to mind for me. @btalamini thoughts?

[white238]

This has always been a complicated section, memory lifecycle wise. We could also add functionality to MFEMSidreDataCollection allow transfer of ownership back to us.

At some point we should revisit restarts as a whole, I am not entirely sure they work at this point, but the concept is what the other customer uses.

[white238]

Either way i feel this is out of the scope of this PR and could be a follow up.

[tupek2]

I good with how it is, and revisiting once we revive restart more.