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

Fix stokes residual postprocessor for purely expensive solves #1848

Merged
merged 1 commit into from
Jul 11, 2017

Conversation

gassmoeller
Copy link
Member

SolverControl::get_history_data throws an exception if asked for non-existent history data (like if we do not make any cheap Stokes solver steps). Make sure there is history data for the postprocessor to use before calling that function. The new test fails with current master, but succeeds with the patch. Still need to update the test results.

@@ -106,7 +107,8 @@ namespace aspect

// If there were expensive iterations add them after a signalling -1.
if ((solver_control_cheap.last_check() == dealii::SolverControl::failure)
&& (solver_control_cheap.last_step() == solver_control_cheap.max_steps()))
&& (solver_control_cheap.last_step() == solver_control_cheap.max_steps())
&& (solver_control_expensive.max_steps() > 0))
Copy link
Contributor

Choose a reason for hiding this comment

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

Would we not get into the same kind of trouble is the cheap solver did not run?

@gassmoeller gassmoeller force-pushed the fix_stokes_residual_output branch 2 times, most recently from ee421cc to 3099ad8 Compare July 11, 2017 16:00
@gassmoeller
Copy link
Member Author

I do not quite understand your comment. The additional check makes sure we only run get_history_data if this part of the solver (cheap/expensive) was actually run. I found a way to simplify the logic though. Could you take another look?

@tjhei
Copy link
Member

tjhei commented Jul 11, 2017

Rene, how does your change interact with the overriden get_history_data in compat.h? In other words, does this work with deal.II 8.5 and 9.0.pre?

@bangerth
Copy link
Contributor

Ah, I misunderstood the code. Thanks for fixing this issue!

@bangerth bangerth merged commit ee81548 into geodynamics:master Jul 11, 2017
@gassmoeller
Copy link
Member Author

@tjhei: I have not tested 9.0 extensively, but the get_history_data() functions are identical, and a quick run of the test showed no difference in the output, so I am reasonably confident it works with both versions.

@gassmoeller gassmoeller deleted the fix_stokes_residual_output branch July 11, 2017 17:17
@tjhei
Copy link
Member

tjhei commented Jul 11, 2017

One more question: is this somehow related to this bugfix:
https://github.com/geodynamics/aspect/pull/1600/files#diff-e6b5753f114df2a6f2169165e60d554eR64

I don't remember why I did that. This is part of #1600 and not merged yet.

@gassmoeller
Copy link
Member Author

Yes, with this PR you will no longer need the resize. We considered doing the resize when I fixed the history_data in deal.II, but decided against it, because a user might think I do not know how many iterations I need, so I will simply set max_steps to 1e9, and then that resize will make a mess 😄.

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

3 participants