-
Notifications
You must be signed in to change notification settings - Fork 96
feat: Multiscale preconditioner #3779
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
base: develop
Are you sure you want to change the base?
Conversation
5b121f2
to
f12bd9f
Compare
@OmarDuran disregard my previous (now deleted) comment, I just noticed that you pushed a commit with the fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@klevzoff thank you for your effort in reviving this functionality.
if( params.multiscale.coarseType == LinearSolverParameters::PreconditionerType::direct ) | ||
{ | ||
params.solverType = LinearSolverParameters::SolverType::direct; | ||
return LAI::createSolver( params ); | ||
} | ||
else if( params.multiscale.coarseType == LinearSolverParameters::PreconditionerType::block ) | ||
{ | ||
GEOS_ERROR_IF_NE_MSG( m_builders.size(), 2, "More than 2 blocks are not supported yet" ); | ||
auto solver = std::make_unique< BlockPreconditioner< LAI > >( params.block ); | ||
for( integer i = 0; i < static_cast< integer >( m_builders.size() ); ++i ) | ||
{ | ||
geos::DofManager::SubComponent comp{ params.block.subParams[i]->multiscale.fieldName, { m_builders[i]->numComp(), true } }; | ||
solver->setupBlock( m_params.block.order[i], { comp }, m_builders[i]->makeCoarseSolver() ); | ||
solver->setProlongation( m_params.block.order[i], m_selectors[i] ); | ||
} | ||
return solver; | ||
} | ||
else | ||
{ | ||
params.preconditionerType = params.multiscale.coarseType; | ||
return LAI::createPreconditioner( params ); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just out of curiosity, how much work would it be to add hypre as an option here? So we could try a hybrid MsRSB+BoomerAMG @klevzoff @castelletto1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't remember if I tested this thoroughly, but I believe this should already work - line 318 will create BoomerAMG (via HyprePreconditioner
) as a coarse-level solver, with parameters specified in the XML (amgXXX
attributes). One can also limit the number of multiscale levels to 1 or 2 (maxLevels
attribute) to leave a sufficiently large coarse problem for AMG to work on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I'll take a look!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice to have this new capability
#ifdef GEOS_USE_METIS | ||
metis::partition( graph.toViewConst(), m_params.graph.metis, m_numPart[0], areaPart.toView() ); | ||
#else | ||
GEOS_THROW( "Attempted to use METIS partition, but GEOSX is not built with METIS support.", InputError ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GEOS_THROW( "Attempted to use METIS partition, but GEOSX is not built with METIS support.", InputError ); | |
GEOS_THROW( "Attempted to use METIS partition, but GEOS is not built with METIS support.", InputError ); |
#ifdef GEOS_USE_SCOTCH | ||
scotch::partition( graph.toViewConst(), m_params.graph.scotch, m_numPart[0], areaPart.toView() ); | ||
#else | ||
GEOS_THROW( "Attempted to use Scotch partition, but GEOSX is not built with Scotch support.", InputError ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GEOS_THROW( "Attempted to use Scotch partition, but GEOSX is not built with Scotch support.", InputError ); | |
GEOS_THROW( "Attempted to use Scotch partition, but GEOS is not built with Scotch support.", InputError ); |
preOrPost="both" | ||
numSweeps="1"/> | ||
<Coarsening | ||
partitionType="cartesian" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curiosity again... how does the method behave with the other types (semi-structured and graph)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a multiscale preconditioner capability to GEOS, introducing the Multiscale Restricted Smoothing Basis (MsRSB) method for solving linear systems with multi-level approaches. The feature expands linear solver options and mesh generation capabilities for improved performance on large-scale problems.
- Adds complete multiscale preconditioner infrastructure with MsRSB implementation
- Enhances VTK mesh generation with structured indexing support
- Updates communication tools to support multiscale mesh operations
Reviewed Changes
Copilot reviewed 76 out of 77 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
src/docs/doxygen/GeosxConfig.hpp | Updates METIS library naming convention from GEOSX to GEOS |
src/coreComponents/schema/schema.xsd* | Adds XML schema support for multiscale and InvariantImmiscibleFluid parameters |
src/coreComponents/physicsSolvers/*.cpp/hpp | Integrates multiscale preconditioner into physics solvers |
src/coreComponents/linearAlgebra/utilities/LinearSolverParameters.hpp | Adds comprehensive multiscale parameter structure |
src/coreComponents/mesh/generators/VTK*.cpp/hpp | Enhances VTK mesh handling with structured indexing |
src/coreComponents/mesh/mpiCommunications/*.cpp/hpp | Extends communication tools for object manager synchronization |
src/coreComponents/linearAlgebra/multiscale/msrsb/*.cpp/hpp | New MsRSB level builder implementation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@@ -170,30 +174,14 @@ stdVector< T > collectUniqueValues( stdVector< T > const & data ) | |||
/** | |||
* @brief Check it the vtk grid is a (supported) structured mesh | |||
* @param[in] mesh a vtk grid | |||
* @return @p true if i@p mesh is structured; @p false otehrwise | |||
* @return @p true if i@p mesh is structured; @p false otherwise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in comment: 'i@p mesh' should be '@p mesh'.
* @return @p true if i@p mesh is structured; @p false otherwise | |
* @return @p true if @p mesh is structured; @p false otherwise |
Copilot uses AI. Check for mistakes.
@@ -352,7 +340,7 @@ buildElemToNodes( AllMeshes & meshes ) | |||
} | |||
|
|||
/** | |||
* @brief Split a mesh by partionning it | |||
* @brief Split a mesh by partitioning it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment was corrected from 'partionning' to 'partitioning' which is proper.
Copilot uses AI. Check for mistakes.
@@ -584,26 +572,30 @@ AllMeshes loadAllMeshes( Path const & filePath, | |||
|
|||
|
|||
/** | |||
* @brief Redistributes the mesh using cell graphds methods (ParMETIS or PTScotch) | |||
* @brief Partition the mesh using cell graph methods (ParMETIS or PTScotch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment was corrected from 'Redistributes' to 'Partition' which better describes the function's purpose.
Copilot uses AI. Check for mistakes.
@@ -1766,9 +1942,11 @@ stdVector< int > getVtkToGeosxPolyhedronNodeOrdering( ElementType const elemType | |||
* @param[in,out] cellBlock The cell block to be written | |||
*/ | |||
void fillCellBlock( vtkDataSet & mesh, | |||
stdVector< vtkIdType > const & cellIds, | |||
Span< vtkIdType const > const cellIds, | |||
CellBlock & cellBlock ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter type changed from stdVector< vtkIdType > const &
to Span< vtkIdType const > const
, which is a good improvement for API design as Span provides a non-owning view that's more flexible and safer.
Copilot uses AI. Check for mistakes.
void writeCells( integer const logLevel, | ||
vtkDataSet & mesh, | ||
const geos::vtk::CellMapType & cellMap, | ||
vtk::CellMapType const & cellMap, | ||
string const & structuredIndexAttributeName, | ||
CellBlockManager & cellBlockManager ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function signature was improved by adding the structuredIndexAttributeName
parameter and changing the cellMap parameter from const geos::vtk::CellMapType &
to vtk::CellMapType const &
for better const-correctness.
Copilot uses AI. Check for mistakes.
ArrayType & dst = this->registerWrapper( wrapper.getName(), std::make_unique< ArrayType >() ).reference(); | ||
// This is a hack since Array's copy ctor does not accept ArrayView source | ||
dst.resize( ArrayType::NDIM, src.dims() ); | ||
std::copy( src.data(), src.data() + src.size(), dst.data() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment acknowledges this is a hack, but the implementation manually copies data from ArrayView to Array. Consider creating a proper constructor or utility function to avoid this pattern appearing elsewhere in the codebase.
std::copy( src.data(), src.data() + src.size(), dst.data() ); | |
copyArrayViewToArray(src, dst); |
Copilot uses AI. Check for mistakes.
Remaining items: