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 Stretched Cube Sphere option to BCS package #801

Merged
merged 29 commits into from
Sep 21, 2023

Conversation

biljanaorescanin
Copy link
Contributor

@biljanaorescanin biljanaorescanin commented Aug 17, 2023

In support of the Hazardous Weather Testbed ( HWT project) @atrayano implemented stretched grid capability. Original implementation he shared was done for old boundary conditions package so it had to be redone for python version.

This current implementation offers these choices:

Choose Stretched_Cubed-Sphere grid option:
Name Stretch Factor Lat Lon Resolution Choices
---------- ------------- ------------ ------------- ---------------------
SG001 2.5 39.5 - 98.35 c270, c540, c1080 and c2160
SG002 3.0 39.5 - 98.35 c1536

Output grid name will have stretched grid identifier for example: CF1080x6-SG001_CF1080x6C

I'm reproducing this this PR set currently staged and used from here: /discover/nobackup/projects/gmao/osse2/stage/BCS_FILES/CF1080x6C_CF1080x6C

@biljanaorescanin biljanaorescanin added enhancement New feature or request 0 diff The changes in this pull request have verified to be zero-diff with the target branch. Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) labels Aug 17, 2023
@biljanaorescanin
Copy link
Contributor Author

biljanaorescanin commented Aug 17, 2023

@gmao-rreichle and @weiyuan-jiang one question:
I wasn't sure do we want to add new identifier with the tile file name or just the output name so now its just at output level. Let me know if I need to change this.
Example tile file is still CF1080x6C_CF1080x6C-Pfafstetter.til and not "CF1080x6C-SG001_CF1080x6C-Pfafstetter.til

@mathomp4
Copy link
Member

I suppose one question I've had is should we be more specific with the naming scheme? That is instead of SG001 something like lat_x_lon_x_stretch?

Also, I think these are for longitude of -98.35 as they are over CONUS.

@biljanaorescanin
Copy link
Contributor Author

biljanaorescanin commented Aug 17, 2023

I suppose one question I've had is should we be more specific with the naming scheme? That is instead of SG001 something like lat_x_lon_x_stretch?

Also, I think these are for longitude of -98.35 as they are over CONUS.

@mathomp4 good catch for -98.35! I will fix this!

I think @gmao-rreichle can comment more but his idea is to leave flexibility if this changes we can easily add other lats /lons. And not have super long name for boundary conditions.

@gmao-rreichle
Copy link
Contributor

I suppose one question I've had is should we be more specific with the naming scheme? That is instead of SG001 something like lat_x_lon_x_stretch?

Also, I think these are for longitude of -98.35 as they are over CONUS.

@mathomp4: As @biljanaorescanin said, the string that identifies the particular Stretched CS Grid would have to be rather long, and we only have a single underscore that separates the atm grid resolution from the ocean grid resolution in the bcs naming convention. Using something like lat_x_lon_x_stretch would introduce additional underscores that would mess up the parsing of the resolution string, and without the underscores the string would be even harder to interpret. But I'm open to suggestions.

@biljanaorescanin
Copy link
Contributor Author

I've tested this and with this last commit I can reproduce all options Bill uses. We can also run all other combinations and CS grid is zero diff.

@biljanaorescanin
Copy link
Contributor Author

This PR was tested after last commit on Friday Sept 15th.
It is zero diff for CS grid and zero diff to what Bill was using for stretched grid. I've tested tripolar grid options as well and those are zero diff.
Only boundary conditions set I can't confirm is zero diff is the new option stretched grid with Tripolar (we don't have reference for it) for example: CF0270x6C-SG001_M6TP1440x1080 but we are able to run it and all looks like its working.

Since all code changes are in boundary conditions this is trivially zero diff for GCM.

@gmao-rreichle gmao-rreichle removed the Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) label Sep 18, 2023
@gmao-rreichle gmao-rreichle marked this pull request as ready for review September 18, 2023 20:07
@gmao-rreichle gmao-rreichle requested review from a team as code owners September 18, 2023 20:07
@gmao-rreichle
Copy link
Contributor

@sdrabenh : This PR is ready for merging. I approved it, and my approval should have counted towards the @GEOS-ESM/surface-preproc-team, but that failed again. I guess @mathomp4 can super-approve, assuming the two of you -- as part of the @GEOS-ESM/python-transition-team -- are ok with the python changes.

@mathomp4
Copy link
Member

Ahh. @gmao-rreichle I just removed you and readded you to @GEOS-ESM/surface-preproc-team . And Github Seems to have liked that.

@gmao-rreichle
Copy link
Contributor

@mathomp4: On the PR landing page, I had just noticed a request for my review, despite my earlier approval and the page showing a green checkmark next to my name. I went ahead and approved again, which I thought then took care of the surface-preproc approval. Not sure whether your action of removing and re-adding me prompted the 2nd review button, or whether it was something else. Also, I do remember that you did the removing/re-adding a few weeks ago when we were in a similar situation. So whatever the removing/re-adding might fix, it doesn't seem to last, sadly. In any case, I think we're good to go - provided there aren't any objections to py changes.

Copy link
Member

@mathomp4 mathomp4 left a comment

Choose a reason for hiding this comment

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

Python looks good. I mean, I assume it was tested.

@sdrabenh sdrabenh merged commit eaede11 into develop Sep 21, 2023
@sdrabenh sdrabenh deleted the feature/borescan_stretched_grid branch September 21, 2023 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch. enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants