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

[GeoMechanicsApplication] Fixed invalid values for default linear solver settings #12815

Merged
merged 4 commits into from
Nov 1, 2024

Conversation

rfaasse
Copy link
Contributor

@rfaasse rfaasse commented Nov 1, 2024

No description provided.

@rfaasse rfaasse requested review from avdg81 and WPK4FEM November 1, 2024 13:19
Comment on lines 78 to 85
"solver_type": "amgcl",
"smoother_type": "ilu0",
"krylov_type": "gmres",
"coarsening_type": "aggregation",
"max_iteration": 100,
"verbosity": 0,
"tolerance": 1.0e-6,
"scaling": false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course we can discuss if these are the best defaults we can think of, I just took the settings of one of our tests. I first tried sparse_lu, but I don't think it's always guaranteed that the LinearSolversApplication is initialized, so not sure if that's the most robust option for a default

Copy link
Contributor

@WPK4FEM WPK4FEM left a comment

Choose a reason for hiding this comment

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

Hi Richard,
I'm probably missing some information here. Your remark was about uppercase not being accepted for AMGCL in the solver type. I don't understand why then all other linear solver settings should be rearranged and the preconditioner_type removed. Preconditioner type means something to me, not all of the other settings ( but that is a gap in my knowledge ).
Regards, Wijtze Pieter

"solver_type": "amgcl",
"smoother_type": "ilu0",
"krylov_type": "gmres",
"coarsening_type": "aggregation",
Copy link
Contributor

Choose a reason for hiding this comment

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

The precconditioner_type is missing here. Maybe it is wise to do the lower case of AMGCL as only change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I reverted everything that wasn't related, just did the lower case amgcl, however then we get the following problem:

RuntimeError: Error: AMGCL Linear Solver : preconditioner_type is invalid!
Currently prescribed preconditioner_type : ILU0Preconditioner
Admissible values are :
    amg
    dummy
    relaxation

So I changed that to amg

Copy link
Contributor

@WPK4FEM WPK4FEM left a comment

Choose a reason for hiding this comment

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

The minimal change and now tested.
Nice.

@rfaasse rfaasse enabled auto-merge (squash) November 1, 2024 15:18
@rfaasse rfaasse merged commit 08033fd into master Nov 1, 2024
11 checks passed
@rfaasse rfaasse deleted the geo/fix-default-linear-solver-settings branch November 1, 2024 17:07
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.

3 participants