Only evaluate necessary solution components during particle property update #6051
Conversation
include/aspect/solution_evaluator.h
Outdated
| * call to reinit(). | ||
| * | ||
| * @p solution_values contains the values of the degrees of freedom. | ||
| * @p evaluation_flags controls if nothing, the solution values, or the gradients should be computed. |
There was a problem hiding this comment.
| * @p evaluation_flags controls if nothing, the solution values, or the gradients should be computed. | |
| * @p evaluation_flags controls if nothing, the solution values, and/or the gradients should be computed. |
| for (unsigned int j=0; j<gradient.size(); ++j) | ||
| gradient[j] = inputs.gradients[i][j]; | ||
| if (get_update_flags(j) & update_gradients) | ||
| gradient[j] = inputs.gradients[i][j]; |
There was a problem hiding this comment.
Can we fill these with NaNs in debug mode if not requested?
| return update_values | update_gradients; | ||
| const auto &component_indices = this->introspection().component_indices; | ||
|
|
||
| if (component >= component_indices.velocities[0] && |
There was a problem hiding this comment.
Can you do introspection.component_masks.velocities[component]==true instead?
|
Good suggestions, I addressed all comments. |
include/aspect/introspection.h
Outdated
| * The component mask for all composition components. | ||
| * This mask selects all compositional fields. | ||
| */ | ||
| ComponentMask composition; |
There was a problem hiding this comment.
Should this be "compositions" to be similar to velocities?
| if (get_update_flags(j) & update_values) | ||
| solution[j] = inputs.solution[i][j]; | ||
| else | ||
| solution[j] = numbers::signaling_nan<double>(); |
There was a problem hiding this comment.
Should we do this only in debug mode for performance reasons or do you think this doesn't matter?
a81b6bd
to
2246cbf
Compare
Yes good point. Changed.
The place you commented on is only the deprecated old interface, in which we allocate one |
2246cbf
to
daea71b
Compare
|
Since this was approved and the changes are minor I am going to merge |
| @@ -459,37 +460,41 @@ namespace aspect | |||
| ++p; | |||
| } | |||
|
|
|||
| const UpdateFlags update_flags = property_manager->get_needed_update_flags(); | |||
|
|
|||
There was a problem hiding this comment.
This is the part of the change that caused #6101. We used to get the update flags as UpdateFlags, but now they are EvaluationFlags obtained via a function argument.
This is one further performance improvement on top of #6020 and something I wanted to implement for a long time. The particle property interface now allows to:
World::local_update_particlesto only copy the requested components into theParticlePropertyInputsobjectIf a model uses particle properties that only need certain solution components (like
IntegratedStrain) this marks another significant speedup of the particle update cost of up to 40% (in my test model).One aspect of the code that is not optimal yet is the number of if-conditions necessary inside
SolutionEvaluator<dim>::evaluate. One would expect that ifFEPointEvaluation::evaluateis called with the evaluation flagsnothingthat the function returns immediately without expensive work. That is however not what happens at the moment, the function call withnothingis only slightly cheaper than a call to evaluatevalues. I will open a separate PR against deal.II to see if we can fix that, but for now the current solution works well for ASPECT.