Platypus to MOOSE migration #29633
Draft
Platypus to MOOSE migration #29633
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… valid input parameters
…lair/MFEMSolversAndPreconditioners Enable user specification of solvers and preconditioners
Note that raw coefficients haven't yet been totally expunged from the code, as that will require some changes to the boundary conditions.
Also improved error handling for undeclared materials.
Tests all run, but they don't all pass
These provide the functionality of the MFEMCoefficient classes.
…terials Implement new interface for materials and coefficients
refs #0
refs #0
loganharbour
force-pushed
the
mfem-conf
branch
from
038f74b to
9a370b9
Compare
|
@alexanderianblair one thing we'd probably want to do is shorten the tests for your kernels if possible |
loganharbour
requested changes
Jan 7, 2025
loganharbour
requested changes
Jan 7, 2025
| * [FESpaces] | ||
| * [] | ||
| */ | ||
| class AddFESpaceAction : public MooseObjectAction |
There was a problem hiding this comment.
Rename AddMFEMFESpaceAction
| * This class implements the action controlling the construction of the | ||
| * required MFEM ProblemOperator. | ||
| */ | ||
| class AddProblemOperatorAction : public Action |
There was a problem hiding this comment.
Rename AddMFEMProblemOperatorAction
| * This class implements the action ensuring the mesh uses the same FE space as | ||
| * the displacement for mesh updates. | ||
| */ | ||
| class SetMeshFESpaceAction : public Action |
Comment on lines
+23
to
+26
| // Name of (the test variable associated with) the weak form that the kernel is applied to. | ||
| std::string _test_var_name; | ||
| std::vector<BoundaryName> _boundary_names; | ||
| mfem::Array<int> _bdr_attributes; |
There was a problem hiding this comment.
Suggested change
| // Name of (the test variable associated with) the weak form that the kernel is applied to. | |
| std::string _test_var_name; | |
| std::vector<BoundaryName> _boundary_names; | |
| mfem::Array<int> _bdr_attributes; | |
| // Name of (the test variable associated with) the weak form that the kernel is applied to. | |
| const std::string _test_var_name; | |
| const std::vector<BoundaryName> _boundary_names; | |
| mfem::Array<int> _bdr_attributes; |
Comment on lines
+19
to
+20
| mfem::Array<int> GetMarkers(mfem::Mesh & mesh); | ||
| mfem::Array<int> _bdr_markers; |
There was a problem hiding this comment.
- Does
_bdr_markersneed to be public? GetMarkers->getMarkers- const on
GetMarkers?
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_); |
There was a problem hiding this comment.
- camel case
- no abbreviation in
GetEssentialBdrMarkers - mesh pointers here should probably all be able to be references?
| #include <mfem.hpp> | ||
| #include "libmesh/restore_warnings.h" | ||
|
|
||
| namespace platypus |
| { | ||
|
|
||
| /// Lightweight adaptor over an std::map from strings to pointer to T | ||
| template <typename T> |
There was a problem hiding this comment.
Lots of camel case methods here.
Is it it not possible to have moose context here to make errors more rich?
| #include "MFEMKernel.h" | ||
| #include "ScaleIntegrator.h" | ||
|
|
||
| namespace platypus |
There was a problem hiding this comment.
- resolve namespace
- camel case methods
- move public data that doesn't need to be public
| /** | ||
| * Set the device to use to solve the FE problem. | ||
| */ | ||
| void setDevice(); |
There was a problem hiding this comment.
We should have a discussion at some point on devices and how to generalize specifying something like a GPU in moose
loganharbour
requested changes
Jan 8, 2025
| -DLAPACK_INCLUDE_DIRS="$PETSC_DIR/include" \ | ||
| -DCMAKE_INSTALL_PREFIX="$MFEM_DIR" \ | ||
| -DMFEM_USE_PETSC=YES \ | ||
| -DPETSC_DIR="$PETSC_DIR" \ |
There was a problem hiding this comment.
Still need to resolve #29635 (comment)
In-tree install of petsc with a PETSC_ARCH requires PETSC_ARCH, and requires all of the PETSC_DIRs for the other deps to be PETSC_DIR/PETSC_ARCH
|
@alexanderianblair is it unsurprising that the MFEM tests diff in parallel? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refs #29632