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 distributions parameters 2-moment scheme #191

Merged
merged 1 commit into from
May 9, 2024
Merged

Conversation

crocicc
Copy link
Contributor

@crocicc crocicc commented May 8, 2024

Purpose

To-do

Content


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

@crocicc crocicc requested a review from trontrytel May 8, 2024 21:01
@crocicc crocicc force-pushed the cc/2M_add_params branch from 6a6831e to ad4ce0a Compare May 8, 2024 21:16
Copy link

codecov bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.50%. Comparing base (d19d043) to head (919e25e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #191   +/-   ##
=======================================
  Coverage   98.50%   98.50%           
=======================================
  Files           2        2           
  Lines         134      134           
=======================================
  Hits          132      132           
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@crocicc
Copy link
Contributor Author

crocicc commented May 8, 2024

I saw that in ClimaParams I cannot define parameters with a negative value. Since \mu for the rain distribution is negative, I defined it here as positive, and then I'll change the signs in the documentation and in the 2-moment computations. Is that okay?

@crocicc crocicc changed the title Cc/2 m add params Add distributions parameters 2-moment scheme May 8, 2024
@trontrytel
Copy link
Member

trontrytel commented May 8, 2024

I saw that in ClimaParams I cannot define parameters with a negative value. Since \mu for the rain distribution is negative, I defined it here as positive, and then I'll change the signs in the documentation and in the 2-moment computations. Is that okay?

Why do you think you can't have negative numbers? I think we do have some negative parameters...

I can try again, maybe it was because there was a space between the - and the number

@crocicc
Copy link
Contributor Author

crocicc commented May 8, 2024

Ok perfect, you're right! Now it's negative!

@crocicc crocicc force-pushed the cc/2M_add_params branch from ef8d2d4 to 821895d Compare May 9, 2024 00:21
Copy link
Member

@trontrytel trontrytel left a comment

Choose a reason for hiding this comment

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

LGTM!

You will also need to change the version in the Project.toml

version = "0.10.5"

to 0.10.6 so that we can make a patch release.

@crocicc crocicc force-pushed the cc/2M_add_params branch from 1d02442 to 919e25e Compare May 9, 2024 17:38
@crocicc crocicc merged commit 49fc081 into main May 9, 2024
9 checks passed
@crocicc crocicc deleted the cc/2M_add_params branch May 9, 2024 17:43
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.

2 participants