search

CliMA / ClimaLand.jl

Clima's Land Model
Apache License 2.0
36 stars 9 forks source link

Preallocate bucket auxs #643

Closed Sbozzolo closed 3 months ago

Sbozzolo commented 3 months ago

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

Sbozzolo commented 3 months ago

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?

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.

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)

I agree!