Hoist and precompute common variables #2673
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
charleskawczynski
force-pushed
the
ck/hoist
branch
from
a8a1266 to
953f65c
Compare
charleskawczynski
force-pushed
the
ck/hoist
branch
2 times, most recently
from
997d1f3 to
fdb0d6c
Compare
charleskawczynski
force-pushed
the
ck/hoist
branch
from
b7907e3 to
bc56346
Compare
Pass thermo state, apply formatter Fixes Use Covariant3Vector Fixes Remove reuse of variable name
charleskawczynski
force-pushed
the
ck/hoist
branch
from
bc56346 to
e99dd52
Compare
charleskawczynski
force-pushed
the
ck/hoist
branch
from
e99dd52 to
0750134
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR hoists and precomputes
ᶜgradᵥ_θ_virt,ᶜgradᵥ_q_tot,ᶜgradᵥ_θ_liq_ice, to simplify some very expensive kernels. A step towards #2530.Let's see how the flame graph updates with this PR.
The flame graphs show an improvement with the kernel but, more interestingly, the gradient-interpolations are pretty expensive. Since this PR hoists those computations, this PR should be more performant since those computations are repeated in the main branch. We should probably look into the performance of the gradient-interpolations, but we can also merge this in case we can't improve it as much as we hoped.