Skip to content
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

Reduce allocations in the canopy update aux #666

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

kmdeck
Copy link
Collaborator

@kmdeck kmdeck commented Jun 17, 2024

Purpose

Reduces allocation in Canopy update_aux. Small improvement in time to solution (13%), but most of the solution time is now in updating the drivers, so this is potentially a larger improvement in RHS functions.

To-do

Content

  1. Instead of computing esat, eair, and then relative humidity and allocating each, I wrote a function to compute the relative humidity, and we now broadcast that over existing fields when updating frac_diff in place.
  2. allocate frac_diff in cache and update in place.
  3. refactor update_photosynthesis
  4. refactor compute_PAR, compute_NIR
  5. refactor compute_absorbances (change structure of canopy cache)

  • I have read and checked the items on the review checklist.

@kmdeck kmdeck self-assigned this Jun 20, 2024
@kmdeck kmdeck force-pushed the kd/reduce_canopy_allocations branch from 25d8aa5 to 83e218d Compare June 20, 2024 22:18
Copy link
Member

@AlexisRenchon AlexisRenchon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, the code looks cleaner like that! Thanks!
Maybe add a line in NEWS.md?
I don't think there is any breaking change, right? (all internal)

@AlexisRenchon
Copy link
Member

What labels would be good for this PR?
performance or code clarity?

@kmdeck kmdeck added the performance improve performance label Jun 21, 2024
@kmdeck
Copy link
Collaborator Author

kmdeck commented Jun 21, 2024

What labels would be good for this PR? performance or code clarity?

I added a "performance" label for now :) and will add in the NEWS md before merging. Thanks!

@kmdeck kmdeck force-pushed the kd/reduce_canopy_allocations branch from c109153 to 88176d7 Compare June 23, 2024 00:37
@kmdeck kmdeck added the Run benchmarks Add this label to run benchmarks on clima label Jun 23, 2024
@kmdeck kmdeck force-pushed the kd/reduce_canopy_allocations branch from 3d96c48 to 0a9e8c0 Compare June 24, 2024 14:40
@kmdeck kmdeck enabled auto-merge June 24, 2024 17:32
@kmdeck kmdeck added this to the Maintenance and Improvements milestone Jun 24, 2024
@kmdeck kmdeck merged commit 984d195 into main Jun 24, 2024
10 checks passed
@juliasloan25 juliasloan25 deleted the kd/reduce_canopy_allocations branch June 24, 2024 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance improve performance Run benchmarks Add this label to run benchmarks on clima
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants