-
Notifications
You must be signed in to change notification settings - Fork 24
Feature/second-order-runge #277
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
base: main
Are you sure you want to change the base?
Conversation
@zebengberg We should probably set the default CoCiP parameters to |
@roger-teoh I have another issue out there #199 -- I think it would be good to change all the default parameters at once in the next release! In particular, I think we should set the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Roger -- this looks great! I made a few small cosmetic suggestions.
radiative_heating_effects: bool = False | ||
radiative_heating_effects: bool = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per Marc's comment, let's change this back to False here, then we'll update in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved. I have also set second_order_runge
to False. We can update in a separate PR.
#: CoCiP time integration using the second-order Runge-Kutta scheme | ||
#: Up until v0.54.3, the time integration step was implemented as a First-order Euler method, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#: CoCiP time integration using the second-order Runge-Kutta scheme | |
#: Up until v0.54.3, the time integration step was implemented as a First-order Euler method, | |
#: Perform CoCiP time integration using the second-order Runge-Kutta scheme. | |
#: If False, a first-order Euler method is used instead. A first-order Euler's method | |
#: was used exclusively in earlier versions of pycontrails. | |
#: | |
#: .. versionadded:: 0.54.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
pycontrails/models/cocip/cocip.py
Outdated
# Second-order Runge-Kutta scheme | ||
if params["second_order_runge"]: | ||
# `contrail_2` calculated in first-order Euler method is now used as intermediate values | ||
contrail_12 = copy.deepcopy(contrail_2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
contrail_12 = copy.deepcopy(contrail_2) | |
contrail_12 = contrail_2.copy() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved.
pycontrails/models/cocip/cocip.py
Outdated
details, see :func:`time_integration_runge_kutta`. | ||
""" | ||
# First-order Euler method | ||
contrail_12 = copy.deepcopy(contrail_1) # Set intermediate values to the initial values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
contrail_12 = copy.deepcopy(contrail_1) # Set intermediate values to the initial values | |
contrail_12 = contrail_1.copy() # Set intermediate values to the initial values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
023bcf9
to
50879e6
Compare
7471d2a
to
b63967a
Compare
Closes #XX
Changes
Breaking changes
Features
Fixes
Internals
Tests
make test
)Reviewer