-
Notifications
You must be signed in to change notification settings - Fork 241
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
Add constant mode settings to Amg_data #6222
Conversation
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.
Thank you.
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.
Looks like iteration numbers are similar and only sometimes better. Maybe not too surprising as meshes are coarse in tests.
Maybe we should run one of the examples with melt transport before/after?
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.
Yes, this looks like a clear bug fix. Can you also add a changelog entry that makes clear we now use the constant modes in the melt solver, just to make users aware results may change?
Do I see this correctly that this change generally reduces the number of iterations in tests, but in some cases increases them?
source/simulator/assembly.cc
Outdated
@@ -525,6 +525,7 @@ namespace aspect | |||
|
|||
Amg_data.smoother_sweeps = 2; | |||
Amg_data.coarse_type = "symmetric Gauss-Seidel"; | |||
Amg_data.constant_modes=constant_modes; |
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.
Sorry to nitpick, but just to keep our code style here consistent, please make this into:
Amg_data.constant_modes=constant_modes; | |
Amg_data.constant_modes = constant_modes; |
a4cfff8
to
f1405fc
Compare
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.
Thanks for the fix! Ready to merge.
Addresses an issue in which constant_mode is created but not used.
#6221
@tjhei