Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Platypus to MOOSE migration #29633

Draft
wants to merge 408 commits into
base: next
Choose a base branch
from
Draft

Platypus to MOOSE migration #29633

wants to merge 408 commits into from

Conversation

lindsayad
Copy link
Member

refs #29632

alexanderianblair and others added 30 commits September 27, 2024 17:55
…lair/hephaestus-cleanup

Remove ProblemBuilders and clean up MFEMProblem
@alexanderianblair
Copy link

@alexanderianblair is it unsurprising that the MFEM tests diff in parallel?

Yes - the tests that check that the output GridFunctions are as expected are based on XMLDiff on the output .vtu files from the first MPI task currently, which won't have all the data from the other tasks in parallel.

@alexanderianblair
Copy link

@alexanderianblair one thing we'd probably want to do is shorten the tests for your kernels if possible

Yup, this should be straightforward by reducing tolerances/default no. of iterations (or just switching to smaller meshes...)

scripts/update_and_rebuild_mfem.sh Show resolved Hide resolved
-DLAPACK_INCLUDE_DIRS="$PETSC_DIR/include" \
-DCMAKE_INSTALL_PREFIX="$MFEM_DIR" \
-DMFEM_USE_PETSC=YES \
-DPETSC_DIR="$PETSC_DIR" \

Choose a reason for hiding this comment

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

In addition to this -DPETSC_DIR is set twice in this set of cmake flags


// clang-format off
//* in weak form
//* (\rho\nabla\times H, \nanbla\times v) + (\mu dH/dt, v) + (dB^e/dt, v) - <(\rho\nabla\times H)\times n, v> = 0

Choose a reason for hiding this comment

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

Typo: nanbla here.

# Build and install mfem
mkdir -p "$MFEM_BUILD_DIR"
cd "$MFEM_BUILD_DIR"
cmake .. \

Choose a reason for hiding this comment

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

This is currently for an MFEM build for CPU only - we'll probably want a script to support users who wish to build for use on GPU too.

Probably out of scope for this PR, but a more generic Platypus build script is under development in Platypus PR #67

Copy link
Member

@loganharbour loganharbour Jan 25, 2025

Choose a reason for hiding this comment

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

Yeah, I have something wrapped up for a GPU environment but didn't wanna do it all at once here. There's already enough to review as is.

Due to the way that we to distribution, MFEM (and conduit) need to be built separately, ahead of time within the moose-dev container layer (which all CI and app distribution use at a later time). We could rely on a build of MFEM from platypus, but it would need to use a MFEM submodule in moose and also support an install

Comment on lines +14 to +23
mfem::Array<int> GetEssentialBdrMarkers(const std::string & name_, mfem::Mesh * mesh_);

void ApplyEssentialBCs(const std::string & name_,
mfem::Array<int> & ess_tdof_list,
mfem::GridFunction & gridfunc,
mfem::Mesh * mesh_);

void ApplyIntegratedBCs(const std::string & name_, mfem::BilinearForm & blf, mfem::Mesh * mesh_);

void ApplyIntegratedBCs(const std::string & name_, mfem::LinearForm & lf, mfem::Mesh * mesh_);

Choose a reason for hiding this comment

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

Agree with the above - should also note that we plan to update BoundaryConditions soon to make integrated BCs / boundary integrators handled more similarly to kernels/domain integrators - i.e. with boundary integrators stored in the EquationSystem, to facilitate easier mesh/FESpace updates.


Preferentially, users should create an `MFEMVariable` with respect to an `MFEMFESpace`:

!listing test/tests/kernels/diffusion.i block=Problem FESpaces Variables

Choose a reason for hiding this comment

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

!listing paths to the test input files in documentation files are incorrect/outdated - these should point within test/tests/mfem now

`MFEMProblem` methods are called by `Actions` during parsing of the user's input file, which add
and/or initialize members of the owned [MFEMProblemData](source/problem/MFEMProblemData.md) struct.
The order in which these actions are executed respects the dependencies declared in
[PlatypusApp](source/base/PlatypusApp.md).

Choose a reason for hiding this comment

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

All documentation paths for docs from Platypus including source should update to source/mfem - documentation is currently failing to create these links during build

@moosebuild
Copy link
Contributor

Job Precheck, step Size check on 2413383 wanted to post the following:

Warning: This PR changes repo size by 26.88 MiB.

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.

8 participants