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

Simplify Newton solver function arguments #6125

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

gassmoeller
Copy link
Member

Still looking into the reason why #6116 changes test results I noticed a number of things that could be simplified around the newton solver. This PR does:

  1. Update documentation that is out of date
  2. Simplifies function arguments, in particular I do no longer assume the calling function already happens to have a copy of a vector if the called function needs a copy. If we need a backup copy of a vector in a function, the function should create and destroy that copy. This may cost 1-2 additional vector copies per nonlinear iteration, but it makes the code much easier to understand. I also think because of this complexity there is a bug in compute_initial_newton_residual that I will address in a follow up.

This PR should not change any test results, it contains only code style improvements.

@gassmoeller gassmoeller force-pushed the simplify_newton_stokes branch 2 times, most recently from fab2030 to 79d0aac Compare November 5, 2024 09:14
@gassmoeller gassmoeller force-pushed the simplify_newton_stokes branch from 79d0aac to e8aae7f Compare November 5, 2024 09:17
Copy link
Member

@MFraters MFraters left a comment

Choose a reason for hiding this comment

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

This makes sense to me, and I like the simplification and the additional documentation. Thanks for working on this!

@MFraters MFraters merged commit 4d576b2 into geodynamics:main Nov 5, 2024
8 checks passed
@gassmoeller gassmoeller deleted the simplify_newton_stokes branch November 6, 2024 21:50
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.

None yet

2 participants