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

ODESolver: Added pre and post iteration hooks #1100

Merged
merged 3 commits into from
Mar 16, 2016
Merged

Conversation

chleh
Copy link
Collaborator

@chleh chleh commented Mar 15, 2016

Said hooks are called before and after each iteration of the nonlinear solver.

I need that functionality for TES in order to be able to drop some nonsensible iteration results.

@chleh chleh added this to the TES Process milestone Mar 15, 2016
@chleh chleh changed the title ODESolber: Added pre and post iteration hooks ODESolver: Added pre and post iteration hooks Mar 15, 2016
sys.assembleMatricesPicard(x_new);
sys.preIteration(iteration, x);

sys.assembleMatricesPicard(x);
Copy link
Member

Choose a reason for hiding this comment

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

Why is assembleMatricesPicard now called with x and not with x_new?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At that point those two vectors have exactly the same values. But I think it is more natural to call the assembly with something old (x) and not with something to be computed (x_new).
That's also more consistent with the Newton case.

@chleh
Copy link
Collaborator Author

chleh commented Mar 15, 2016

Sorry for the big change, but I noticed that the pre and post iteration hooks must be put in the EquationSystem interface s.t. both ODESystem and NonlinearSystem inherit them.

@TomFischer
Copy link
Member

👍

}
else
{
// TODO could be solved in a better way
Copy link
Member

Choose a reason for hiding this comment

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

Since there are many todos (in this PR also the merged PR), I think it is better to use doxygen command \todo, which can generates a todo list, to remind one what have to do.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suggest to change all TODO tags to \todo in a separate PR, and then all at once for all of OGS6, if that's important.

@wenqing
Copy link
Member

wenqing commented Mar 16, 2016

👍

@endJunction
Copy link
Member

Looks OK.

chleh added a commit that referenced this pull request Mar 16, 2016
ODESolver: Added pre and post iteration hooks
@chleh chleh merged commit 6c0d9f3 into ufz:master Mar 16, 2016
@chleh chleh deleted the pre-post-iter branch March 16, 2016 13:47
@chleh
Copy link
Collaborator Author

chleh commented Mar 16, 2016

Sorry, I forgot to check that I had to squash commits before merging.

@ogsbot
Copy link
Member

ogsbot commented Jun 19, 2020

OpenGeoSys development has been moved to GitLab.

See this pull request on GitLab.

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.

5 participants