Skip to content

Conversation

weiyuan-jiang
Copy link
Contributor

No description provided.

@weiyuan-jiang weiyuan-jiang requested a review from a team as a code owner July 18, 2025 17:24
Copy link

This PR is being prevented from merging because you have not added one of our required labels: 0 diff, 0 diff trivial, Non 0-diff, 0 diff structural, 0-diff trivial, Not 0-diff, 0-diff, automatic, 0-diff uncoupled. Please add one so that the PR can be merged.

@weiyuan-jiang weiyuan-jiang added 0 diff The changes in this pull request have verified to be zero-diff with the target branch. enhancement New feature or request labels Jul 18, 2025
@weiyuan-jiang
Copy link
Contributor Author

This PR should go with this PR GEOS-ESM/GEOSgcm_GridComp#1129

Copy link
Contributor

@gmao-rreichle gmao-rreichle left a comment

Choose a reason for hiding this comment

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

@weiyuan-jiang : I added one commit (4c5441c; please review) and a comment, see below.

@biljanaorescanin
Copy link
Contributor

@weiyuan-jiang last commit, reinstating subroutine fixed the issue.

@gmao-rreichle
Copy link
Contributor

@weiyuan-jiang last commit, reinstating subroutine fixed the issue.

@weiyuan-jiang , @biljanaorescanin : I assume you ran into problems because are running GEOSldas with the cnclm51 branch, where get_gridname() is used. I had commented out get_gridname() because it was not used in GEOS_Util. I hadn't realized that it is used in ldas.py of GEOSldas_GridComp. While re-instating get_gridname() in GEOS_Util fixes the issue, I don't think it's the right solution. In ldas.py, get_gridname() is needed for HISTORY pre-processing, which has nothing to do with remap_restarts or GEOS_Util. I think we should move get_gridname to ldas.py. @weiyuan-jiang, does this make sense?

@gmao-rreichle
Copy link
Contributor

PS: Or maybe get_gridname() would fit better into the make_bcs package? It's really some sort of reader of the *.til file. I don't know if we have a full py reader for the *.til file. If that exists, perhaps it could be modified to optionally only read the "grid_name" info. Also, the *.til file contains two grid names, the atm grid and the ocean grid. We should probably be clearer about get_gridname() only reading the name of the atm grid.

@weiyuan-jiang
Copy link
Contributor Author

PS: Or maybe get_gridname() would fit better into the make_bcs package? It's really some sort of reader of the *.til file. I don't know if we have a full py reader for the *.til file. If that exists, perhaps it could be modified to optionally only read the "grid_name" info. Also, the *.til file contains two grid names, the atm grid and the ocean grid. We should probably be clearer about get_gridname() only reading the name of the atm grid.

Since it is called only by ldas_setup, I can move it to setup_utills

@weiyuan-jiang weiyuan-jiang force-pushed the feature/wjiang/cnclm51 branch from d1a51fa to 6ee5312 Compare August 15, 2025 14:47
@gmao-rreichle gmao-rreichle changed the title add cnclm51 remapping add cnclm51 remapping; provide default "zoom" value for remap_restarts yaml file Aug 20, 2025
@biljanaorescanin
Copy link
Contributor

Zero diff testing with remap_package passed.

gmao-rreichle
gmao-rreichle previously approved these changes Aug 25, 2025
@gmao-rreichle gmao-rreichle dismissed stale reviews from biljanaorescanin and themself via b909338 August 25, 2025 17:16
@gmao-rreichle gmao-rreichle requested a review from a team as a code owner August 25, 2025 17:16
@sdrabenh sdrabenh merged commit 9278c5c into main Aug 25, 2025
11 checks passed
@sdrabenh sdrabenh deleted the feature/wjiang/cnclm51 branch August 25, 2025 18:55
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.

4 participants