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

Wip on summing all rain tendencies #372

Merged
merged 1 commit into from
Oct 12, 2021
Merged

Wip on summing all rain tendencies #372

merged 1 commit into from
Oct 12, 2021

Conversation

trontrytel
Copy link
Member

@trontrytel trontrytel commented Oct 10, 2021

This finished the rain cleanup. All tendencies are calculated and then applied in one place.

@charleskawczynski - lmk what other changes to names/arrays etc you want done at this point. Or is it enough to leave for now and wait for a new timestepper?

@trontrytel trontrytel self-assigned this Oct 10, 2021
@trontrytel trontrytel force-pushed the aj/rain_tendencies_apply branch 2 times, most recently from 8c71919 to 1ebcab1 Compare October 12, 2021 17:18
@trontrytel trontrytel marked this pull request as ready for review October 12, 2021 17:18
@trontrytel
Copy link
Member Author

There are some tiny changes for Rico case. But the profiles look almost indistinguishable to me

Copy link
Member

@charleskawczynski charleskawczynski left a comment

Choose a reason for hiding this comment

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

LGTM

@trontrytel trontrytel force-pushed the aj/rain_tendencies_apply branch 2 times, most recently from 6491c6f to d08f18b Compare October 12, 2021 19:11
TS::TimeStepping,
)
@inbounds for k in real_center_indices(rain_var.grid)
rain_var.QR.values[k] +=
Copy link
Member

Choose a reason for hiding this comment

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

should we have a max(...,0) here to ensure that QR is positive?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we do it here we would not be conserving mass anymore. Since the same tendencies need to be subtracted from qt. I think a better strategy is to have your scheme not break with small negatives for qr

@trontrytel
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 12, 2021

@bors bors bot merged commit e305445 into main Oct 12, 2021
@bors bors bot deleted the aj/rain_tendencies_apply branch October 12, 2021 20:16
charleskawczynski referenced this pull request Oct 16, 2021
414: Bump version r=charleskawczynski a=charleskawczynski



Co-authored-by: Charles Kawczynski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants