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

Add deep convection to gcm-driven SCM setup #3364

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

costachris
Copy link
Member

Add relaxation timescale for deep convection in gcm-driven SCM setup

@costachris costachris force-pushed the cc/add_deep_conv_gcmdriven branch from 0ff34b7 to 7733cb7 Compare October 4, 2024 21:34
@costachris costachris requested a review from szy21 October 4, 2024 21:34
@costachris costachris force-pushed the cc/add_deep_conv_gcmdriven branch from 7733cb7 to 0341135 Compare October 4, 2024 22:09
@costachris costachris marked this pull request as ready for review October 4, 2024 22:12
@costachris costachris requested a review from Sbozzolo October 4, 2024 22:56
@szy21
Copy link
Member

szy21 commented Oct 4, 2024

The parameters change looks good to me. Gabriele can take a look at the code change.

Copy link
Member

@Sbozzolo Sbozzolo left a comment

Choose a reason for hiding this comment

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

Can you turn forcing_type into a julia type and use dispatch instead of matching strings?

@costachris costachris force-pushed the cc/add_deep_conv_gcmdriven branch from 0341135 to 990f3b1 Compare October 6, 2024 20:03
Copy link
Member

@Sbozzolo Sbozzolo left a comment

Choose a reason for hiding this comment

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

Code looks good (altough, I think that it is more efficient to dispatch over the first argument). Could you please add some documentation, docstrings, and expand upon what the config external_forcing_type is?

Your help message for external_forcing_type is "External forcing type", which doesn't teach much

@szy21
Copy link
Member

szy21 commented Oct 7, 2024

Actually, I think we should add these time scales to parameters instead of having a type. No need to change it now, but whoever uses it or changes it again for other types (e.g. reanalysis) should use parameters instead I think.

@costachris costachris force-pushed the cc/add_deep_conv_gcmdriven branch from 990f3b1 to 67b0d4c Compare October 7, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants