-
Notifications
You must be signed in to change notification settings - Fork 237
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
use sundials kinsol for diffusion dislocation creep #5698
base: main
Are you sure you want to change the base?
Conversation
/rebuild |
@bangerth, would you mind having a look at this? Total average runtime for |
Yes, will look at this. This is exciting! |
while (std::abs(log_strain_rate_residual) > log_strain_rate_residual_threshold | ||
&& stress_iteration < stress_max_iteration_number) |
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.
This is your current stopping criterion. Could you check how many iterations you are using in this loop (in total or on average) in one of your benchmarks, and compare that against the number of iterations you're spending in KINSOLS? I think that might clarify why the code is so much slower.
You can also directly time individual sections by querying simulator_access.get_computing_timer()
. You can take what's in particles/world.cc
as an example of how you can put a timing section around a piece of code.
Solving Stokes system... 16+0 iterations. | ||
Relative nonlinear residuals (temperature, compositional fields, Stokes system): 1.68721e-16, 1.05609e-16, 0.000715698 | ||
Relative nonlinear residual (total system) after nonlinear iteration 2: 0.000715698 | ||
Solving Stokes system... 21+0 iterations. | ||
Relative nonlinear residuals (temperature, compositional fields, Stokes system): 1.68721e-16, 1.05609e-16, 0.000716824 | ||
Relative nonlinear residual (total system) after nonlinear iteration 2: 0.000716824 |
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.
It's a bummer that (at the moment) you need more iterations, but a good sign that the results are essentially identical.
e96ece1
to
a0a09e4
Compare
@bangerth As requested, I've created a little standalone file by modifying step-77.cc. It exhibits the same behaviour as seen in |
At the very least, I can confirm the issue with the two nearby points:
|
For reference, in the first call, this is the call trace:
whereas in the second (with the almost identical
The third call is from here:
Call number 4 is again from here:
And the last call is from
I will have to install the source code of SUNDIALS to make sense of this. |
I decided to ask for help from my SUNDIALS friends at LLNL/sundials#500. |
For completeness, this is the minimal testcase:
|
@bobmyhill Is it expected that in your example, |
Hi @bangerth, yes, the function is almost piecewise linear (with a smoothed kink). |
a0a09e4
to
97d618f
Compare
97d618f
to
39f923e
Compare
@bangerth I guess this is ready from my side. I think the slowdown is probably just the number of iterations for this particular case. One thought - would it be possible / worth it to move any of the KINSOL setup out of calculate_isostrain_viscosities? additional_data could be moved out, but I'm not sure anything else can. |
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.
Not sure what will happen with this PR, but just in case I think I found a case of unnecessary copies in the residual function.
const Rheology::DiffusionCreepParameters diffusion_creep_parameters, | ||
const Rheology::DislocationCreepParameters dislocation_creep_parameters, |
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.
I think you want to hand over these two structures by reference to avoid the copies.
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.
Some (small) comments.
// We use KINSOL to find the second invariant of the stress tensor that satisfies the total strain rate. | ||
SUNDIALS::KINSOL<Vector<double>>::AdditionalData additional_data; | ||
additional_data.strategy = dealii::SUNDIALS::KINSOL<>::AdditionalData::newton; | ||
additional_data.function_tolerance = log_strain_rate_residual_threshold; | ||
additional_data.maximum_non_linear_iterations = stress_max_iteration_number; | ||
additional_data.maximum_setup_calls = 1; |
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.
You can definitely move this out of the loop over the chemical composition fields.
@@ -54,10 +55,10 @@ namespace aspect | |||
const double edot_ii = std::max(std::sqrt(std::max(-second_invariant(deviator(strain_rate)), 0.)), | |||
min_strain_rate); | |||
const double log_edot_ii = std::log(edot_ii); | |||
|
|||
double log_strain_rate_deriv; |
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.
I would put that declaration right above the lambda functions that use it to minimize the scope of the variable.
// KINSOL works on vectors and so the scalar (log) stress is inserted into | ||
// a vector of length 1 | ||
Vector<double> log_stress_ii(1); | ||
log_stress_ii[0] = std::log(stress_ii); |
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.
This is your starting guess. How is it chosen? Based on the current log stress? It might be worth trying the output of the previous loop iteration, under the assumption that what you found in the previous iteration is a good starting guess for the current iteration. Not sure whether that's true, since you're not looping over quadrature points, for example, but a thought.
[&](const Vector<double> ¤t_log_stress_ii, | ||
Vector<double> &residual) | ||
{ | ||
std::tie(residual(0), log_strain_rate_deriv) = compute_log_strain_rate_residual_and_derivative(current_log_stress_ii[0], pressure, temperature, diffusion_creep_parameters, dislocation_creep_parameters, log_edot_ii); |
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.
Maybe break this line somewhere.
603bf73
to
3b01b3a
Compare
This PR replaces the handrolled Newton scheme in diffusion-dislocation creep with KINSOL routines.