-
Notifications
You must be signed in to change notification settings - Fork 277
[Core] Add PMultigridBuilderAndSolver
#13276
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
Conversation
|
My only concern is that this conflicts with the current developments of @rubenzorrilla |
|
I cannot comply with future plans that aren't fleshed out yet. After the new system gets introduced, |
Just informing you to minimize your workload. Great job anyway, if possible try to bring changes in smaller PR. For example sparse utilities can be added to utilities folder in a independent PR, etc... |
|
Yes I feel a bit guilty about the weight of this PR, but none of the components make sense in a vacuum. For example, I have a hard time imagining anyone would use any function The whole PR would be much more compact if I just wrote everything in one file, but then it'd be difficult to adapt to the new strategies and it would be ~2.5k lines in one file. |
Okay, ping once is ready |
…into core/p-multigrid
…into core/p-multigrid
sunethwarna
left a comment
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.
some minor comments. Nice work @matekelemen.
|
|
||
| for (auto i_entry=i_entry_begin; i_entry<i_entry_end; ++i_entry) { | ||
| const auto i_dof = rRelationMatrix.index2_data()[i_entry]; | ||
| const Dof<typename TDense::DataType>& r_dof = *(itDofBegin + i_dof); |
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.
isn't this little bit dangerous? I would atleast do a debug level check whether the r_dof exists. Developer may encounter lots of bugs in giving these iterators. Production code may not require these checks.
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 add a check that makes sure that the number of Dofs matches the number of columns in the matrix.
I wouldn't add checks in the middle of hot loops like this one though, debug build or otherwise. The only way it could fail is an ill-formed input matrix (column indices out of bound), but that would have to come from very far away.
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.
Fair enough, but we need to add the checks in the Check methods. I had to debug similar scenarios multiple times within last 6 months.
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.
BuilderAndSolver::Check only takes a ModelPart so there's no way for me to check the linear system there.
...lving_strategies/builder_and_solvers/p_multigrid/augmented_lagrange_constraint_assembler.hpp
Outdated
Show resolved
Hide resolved
kratos/solving_strategies/builder_and_solvers/p_multigrid/constraint_utilities.hpp
Show resolved
Hide resolved
kratos/solving_strategies/builder_and_solvers/p_multigrid/diagonal_scaling.cpp
Show resolved
Hide resolved
kratos/solving_strategies/builder_and_solvers/p_multigrid/linear_multifreedom_constraint.cpp
Show resolved
Hide resolved
kratos/solving_strategies/builder_and_solvers/p_multigrid/noop_constraint_assembler.hpp
Show resolved
Hide resolved
kratos/solving_strategies/builder_and_solvers/p_multigrid/p_multigrid_builder_and_solver.cpp
Outdated
Show resolved
Hide resolved
…into core/p-multigrid
|
@sunethwarna ready for another round |
This PR depends on: #13272.
Add a new
BuilderAndSolverand its related infrastructure that implements a multigrid preconditioner exploiting the properties of higher order elements.compound.mp4
My recommendation to the reviewers is to begin with
p_multigrid_builder_and_solver.hpp, which is the entry point to the entire system.Exposed Features
Don't worry, most of the new files/classes you'll see are meant exclusively for internal use in
PMultigridBuilderAndSolverand should not pollute the rest of Kratos. With that said, there are two new classes that should be made accessible in Kratos:PMultigridBuilderAndSolveris the main class that everything else plugs into. Depending on how it is configured, it can also be used as a replacement forResidualBasedBlockBuilderAndSolveras it shared most of the logic and some of the code. Some of its features are not supported (yet) though (such as rebuilding only the RHS).MultifreedomConstraintis a constraint class with a slightly different behavior thanMasterSlaveConstraint. This was necessary to support nonlinear constraints. The new B&S supports bothMultifreedomConstraintandMasterSlaveConstraint.Internal Abstractions
ConstraintAssembleris similar in its purpose toBuilderAndSolver, but it is exclusively responsible for assembling the constraint equations and using them to impose the constraints on the main system.Scalingis a utility class that handles diagonal scaling either as a constant, or as a function of some norm of the LHS matrix' diagonal.Sparse utilities are reponsible for assembling from the local to the global system, computing the topology of sparse matrices, and imposing dirichlet conditions.
Constraint utilities handle common tasks during constraint assembly.
Multigrid utilities are responsible for computing the the preconditioner's restriction operator.
Tests
Testing a B&S needs an application, so I'll put them in a subsequent PR in structural mechanics.
Notes
@rubenzorrilla I tried to extract as much functionality into other classes and utilities as I could to make an eventual change to the new schemes simpler.