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 JRA Repeat Year Forcing data modes #3691

Merged
merged 5 commits into from
Sep 23, 2020

Conversation

alperaltuntas
Copy link
Member

Brings in three JRA RYF (repeat year forcing) datm and drof data modes: RYF8485, RYF9091, RYF0304.
These new forcings are described in: https://doi.org/10.1016/j.ocemod.2019.101557

Test suite: G cases with all three RYF modes. scripts_regression_test
Test baseline: n/a
Test namelist changes: n/a
Test status: n/a

User interface changes?: np

Update gh-pages html (Y/N)?:

Code review:

@klindsay28
Copy link
Collaborator

Should CORE_RYF.*GISS.SWDN have the same value for strm_offset and tintalgo as CORE_IAF_JRA.*GISS.SWDN (i.e., -5400 and coszen respectively)? This would lead to the same treatment of swdn in the repeat year forcing as in the full-length IAF_JRA forcing (which I would expect).

If so, this might require splitting CORE_RYF.*GISS into CORE_RYF.*GISS.LWDN and CORE_RYF.*GISS.SWDN.

Copy link
Contributor

@jedwards4b jedwards4b left a comment

Choose a reason for hiding this comment

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

I would like @mvertens to review this instead of me, she is much more familiar with data models.
This should also be added to the new cdeps repository.

Copy link
Contributor

@mvertens mvertens left a comment

Choose a reason for hiding this comment

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

I am fine with the code that exists. However, @klindsay28 brings up an important point that needs to be addressed before the PR is accepted. @alperaltuntas @klindsay28 - I would like to also have CDEPS be added as another place that changes and tests for data models need to be made.

@whokim
Copy link

whokim commented Aug 26, 2020

I haven't realized this before. This is all inherent from CORE2_NYF.GISS, which is set to 0 and linear for offset and tintalgo, respectively. So, @klindsay28, do you thinks that these should be -5400 and coszen?

@klindsay28
Copy link
Collaborator

In compset G experiments, i.e., CORE2_NYF,swdn is a daily average, ATM_NCPL=24, and tintalgo=linear. So datm is taking daily averages and linearly interpolating to hourly values. This doesn't make sense to me for swdn, as the resulting hourly fields passed to the coupler lack a diurnal cycle.

Note that back when this compset was created, the ocean was typically coupled daily, OCN_NCPL=1, so ignoring the diurnal cycle of swdn didn't matter that much. If it had been present, it would have been averaged out. Note that it did matter for the sea ice model. I'm also not sure if tintalgo=coszen existed back then. We later starting imposing a diurnal cycle within the ocean model, which worked reasonably well when the ocean got a daily average.

However, we now use OCN_NCPL=24, so the ocean seas the lack of the diurnal cycle. So yes, I think swdn should have tintalgo=coszen. I think this would require splitting CORE2_NYF.GISS into CORE2_NYF.GISS.LWDN and CORE2_NYF.GISS.SWDN.

The offset value is a bit more subtle. When you use tintalgo=coszen, the time values are expected to be the beginning of the interval. In JRA files, time is in the middle of 3-hour intervals. So subtracting 1.5 hours = 5400 sec yields a time value at the beginning of the interval. This is why offset=-5400 for JRA. However, in the CORE2_NYF GISS files, time is in the middle of a daily interval. So we would want offset=-43200 for CORE2_NYF.GISS.SWDN.

I'm not sure if changes to CORE2_NYF.GISS belong in the PR, or if they deserve a PR of their own. If we do decide to propose changing CORE2_NYF.GISS, whether in this PR or another one, I do think that such changes should be highlighted to E3SM developers, such as @jonbob.

@alperaltuntas
Copy link
Member Author

Thanks! I'll update this PR and apply the changes suggested by @klindsay28.

@mvertens
Copy link
Contributor

mvertens commented Aug 26, 2020 via email

@alperaltuntas
Copy link
Member Author

@klindsay28, Can you confirm the latest changes look okay?

Copy link
Contributor

@jonbob jonbob left a comment

Choose a reason for hiding this comment

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

This should not affect ongoing E3SM JRA usage, and should give us additional configurations once @klindsay28's comments are addressed

@klindsay28
Copy link
Collaborator

@alperaltuntas, the latest changes look good to me.
Thanks for addressing this concern.

@alperaltuntas
Copy link
Member Author

Thanks! This is ready to be merged now.

@fischer-ncar
Copy link
Contributor

fischer-ncar commented Sep 2, 2020

Do we want to get this PR into the CESM2.2 release?

@mvertens
Copy link
Contributor

mvertens commented Sep 3, 2020

@alper - I agree with you. This does not have to be in the CESM2.2 release.

@jedwards4b jedwards4b merged commit 2499661 into ESMCI:master Sep 23, 2020
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.

None yet

8 participants