Remove allocation and clipping of Y#1748
Merged
Merged
Conversation
kmdeck
commented
May 18, 2026
| θ_l = soil_moisture(model.drivers.met, p, Y, t, z) | ||
| θ_i = soil_ice(model.drivers.met, p, Y, t, z) | ||
| Csom = @. lazy(max(Y.soilco2.SOC, 0)) # Clamp SOC to non-negative (lazy, no allocation) | ||
| Csom = Y.soilco2.SOC |
Member
Author
There was a problem hiding this comment.
I dont think the original line actually clipped Y here, but I am not sure.
AlexisRenchon
approved these changes
May 18, 2026
AlexisRenchon
left a comment
AlexisRenchon
left a comment
Member
There was a problem hiding this comment.
If the longrun looks okay, let's merge it! (I can have a look when it is out)
Member
Author
no NaNs! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Purpose
This PR removes direct clipping of Y, which would prevent us from conserving carbon (and oxygen). It also removes one allocation of a 3D field. (Im not sure if that could be the culprit for the recent AMIP slowdown @juliasloan25 ? this allocation was added between 1.8.0 and 1.8.1)
I also adjusted some comments to reflect that some of the clipping, while allowing the simulation to carry on with "physical" values, is going to correspond to an incorrect solution to the equations (it will not agree with the solution you would get with a stable simulation). This is problematic because in these columns the solution becomes controlled by clipping etc and not by the physics. we can try and fix this in the future.
Ill try the long run!
To-do
Content