-
Notifications
You must be signed in to change notification settings - Fork 10
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
Preallocate bucket auxs #643
Conversation
193fabe
to
cca1d39
Compare
cca1d39
to
1acf6c0
Compare
f0acb88
to
a776475
Compare
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 this looks fine! I am just wondering if any unit tests need to change to reflect these additions. How are we determining that the behavior doesnt change with this PR?
One other comment is that I think at this point we dont need to improve the bucket performance on its own, but should make changes that improve the performance of all models (e.g. allocations in turbulent_fluxes
, net_radiation
, etc)
a776475
to
748634b
Compare
748634b
to
cdcfa5f
Compare
This is a good question. The PR only moves code around, so it things were previously properly tested, they will stay properly tested. I added unit tests to check that things should be consistent.
I agree! |
This PR preallocates some of the Fields that are created when computing the bucket tendency.
New time: 3.6s
https://buildkite.com/clima/climaland-benchmark/builds/355#018fef98-5edd-4f39-9cbd-a11a529eba6f