-
Notifications
You must be signed in to change notification settings - Fork 127
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
Radiation changes #54
base: master
Are you sure you want to change the base?
Conversation
I didn't intend to pass across the `Add T21 spectral resolution to experiment.py' commit, as it has a missing curly brace after the resolution definition. How do I remedy this? |
You can commit a fix to the same branch and the PR will pick it up
… On 29 Jun 2018, at 19:20, Neil Lewis ***@***.***> wrote:
I didn't intend to pass across the `Add T21 spectral resolution to experiment.py' commit, as it has a missing curly brace after the resolution definition. How do I remedy this?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Issue fixed. I left in the T21 resolution definition, but added the missing brace, as I figured it could be useful to someone at some point. |
Great. Yes - definitely a useful addition, thanks for doing that |
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.
Overall looks good.
- There is a core dump that is in the commit? Please delete. How big is this file? If it's huge we might need to scrub it from the history.
- Are the land masks generic, or specific to you? If specific, please remove or document.
- See the inline comments below. I think you have committed a couple of temp files that could do with being removed.
@jamesp My apologies for some of the garbage thats ended up in this commit! I'll work through cleaning it up in the next week or two. |
Conflicts: src/extra/python/isca/experiment.py
@jamesp I think I've tidied this up now so should be ready to merge 😄 |
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.
Nice one @ntlewis! Can you just confirm you've sorted all the merge artefacts in two_stream_gray_rad?
A few changes are in here:
Multiply insolation in rrtm_radiation and two_stream_gray_rad by rrsun so that non-zero eccentricity has an effect.
Add deallocation for h2o input file interpolation to rrtm_radiation (was missing before).
Add option to prescribe h2o to two_stream_gray_rad from input file using the interpolator.
Add Nakajima 1992 definition of longwave optical depths to two_stream_gray_rad as additional gray radiation scheme option. Ref: https://journals.ametsoc.org/doi/abs/10.1175/1520-0469(1992)049%3C2256:ASOTGE%3E2.0.CO;2.
All additions have been tested.