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

IJ indices for stretched grid and bubble up error from MAPL_getHorzIJindex subroutine #2851

Merged
merged 14 commits into from
Oct 8, 2024

Conversation

weiyuan-jiang
Copy link
Contributor

@weiyuan-jiang weiyuan-jiang commented May 28, 2024

Types of change(s)

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Trivial change (affects only documentation or cleanup)

Checklist

  • Tested this change with a run of GEOSgcm
  • Ran the Unit Tests (make tests)

Description

Stop the program when the grid is not a normal CS grid but computes the IJindex. It partially addresses issue #2850 . Eventually, I think the Grid class should have information about stretch and the ij index is computed according to the info the grid provides

Related Issue

@weiyuan-jiang weiyuan-jiang requested a review from a team as a code owner May 28, 2024 19:43
@mathomp4
Copy link
Member

The main problem is, if we commit this, then we can't run the stretched grid with GOCART, right? And, well, I'm not sure if that's desired.

We might need to support some sort of "slower"/"dumber" IJ method if running stretched grid.

@mathomp4 mathomp4 marked this pull request as draft May 28, 2024 22:11
@mathomp4
Copy link
Member

I've drafted this PR until @tclune can have a think on it as well.

Copy link

github-actions bot commented Sep 7, 2024

This PR has been automatically marked as stale because it has not had activity in the last 60 days. If there are no updates within 7 days, it will be closed. You can add the ":hourglass: Long Term" label to prevent the stale action from closing this issue.

@github-actions github-actions bot added the ❄️ Stale This issue has been marked stale label Sep 7, 2024
@mathomp4 mathomp4 removed the ❄️ Stale This issue has been marked stale label Sep 19, 2024
@weiyuan-jiang weiyuan-jiang added the 0 Diff The changes in this pull request have verified to be zero-diff with the target branch. label Sep 20, 2024
@mathomp4
Copy link
Member

I can confirm this definitely works! I ran a stretched case and failed as expected.

@weiyuan-jiang weiyuan-jiang changed the title bubble up error from MAPL_getHorzIJindex function IJ indices for stretched grid and bubble up error from MAPL_getHorzIJindex subroutine Sep 30, 2024
@mathomp4
Copy link
Member

Hmm. GNU is not happy:

/root/project/GEOSgcm/src/Shared/@MAPL/base/Base/Base_Base_implementation.F90:3458:33:

 3458 |        latRe = half_pi*sign(1.0, Zz)
      |                                 1
Error: 'b' argument of 'sign' intrinsic at (1) must be the same type and kind as 'a'
make[2]: *** [src/Shared/@MAPL/base/CMakeFiles/MAPL.base.dir/build.make:1094: src/Shared/@MAPL/base/CMakeFiles/MAPL.base.dir/Base/Base_Base_implementation.F90.o] Error 1

@weiyuan-jiang
Copy link
Contributor Author

The function for reverse Schmidt transform is added and the indices can be calculated now. But I have not run the gcm with stretched grid. How can I do the setup and run? @mathomp4

@mathomp4
Copy link
Member

The function for reverse Schmidt transform is added and the indices can be calculated now. But I have not run the gcm with stretched grid. How can I do the setup and run? @mathomp4

@weiyuan-jiang My makeoneday script should work for c270 or c540. Just choose Ostia-CS ocean as well.

keep the types of "sign" arguments consistent
@mathomp4
Copy link
Member

mathomp4 commented Oct 1, 2024

I just tried a C270 run and:

 accurate_lon:    5.32325421858271
 accurate_lat:  -0.483265549030082
 edge_lat    :   0.350599277377354
 edge point  :           30
 Error: It could be
   1)Grid is NOT gnomonic_ed;
   2)lats lons from MAPL_GridGetCorners are NOT accurate (single precision from
 ESMF)

and I also saw:

 Error: Grid should have pi/18 shift

@tclune
Copy link
Collaborator

tclune commented Oct 1, 2024

Oh yes. The pi/18 shift does not happen for the stretched grid. ...

@weiyuan-jiang
Copy link
Contributor Author

This PR also addresses issue #3060

@weiyuan-jiang weiyuan-jiang marked this pull request as ready for review October 3, 2024 13:06
@mathomp4
Copy link
Member

mathomp4 commented Oct 8, 2024

@weiyuan-jiang I just tried building this and running it at C270 and I get:

 NOT using buffer I/O for file: tavg3d_aer_p.rcx
 NOT using buffer I/O for file: HISTORY.rc

 Freq: 00060000  Dur: 00060000  TM:   -1  Collection: geosgcm_prog
forrtl: severe (41): insufficient virtual memory
Image              PC                Routine            Line        Source
libMAPL.pfio.so    000014C936ACD0B9  pfio_variablemod_         139  Variable.F90
libMAPL.base.so    000014C9382C148C  mapl_latlongridfa        1821  MAPL_LatLonGridFactory.F90
libMAPL.griddedio  000014C938AFDFC7  mapl_griddediomod         170  GriddedIO.F90
libMAPL.history.s  000014C93A1533B0  mapl_historygridc        2470  MAPL_HistoryGridComp.F90

and it's dying in parse_dimensions() in Variable. I'll try a debug run to see if maybe my optimized build is doing weird things.

@mathomp4
Copy link
Member

mathomp4 commented Oct 8, 2024

Update: maybe this is on me. My Debug build worked. I'm going to nuke my release build and build from scratch.

@mathomp4
Copy link
Member

mathomp4 commented Oct 8, 2024

Okay. It was me. I can run C270 now without errors, etc.

@mathomp4
Copy link
Member

mathomp4 commented Oct 8, 2024

Oh. I guess I should test with v11. I just did it with v12.

Also, ping @mmanyin that the stretch grid might be okay again...

Copy link
Collaborator

@tclune tclune left a comment

Choose a reason for hiding this comment

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

If it works, it works.

@mathomp4 mathomp4 merged commit 3b4fd72 into develop Oct 8, 2024
36 checks passed
@mathomp4 mathomp4 deleted the feature/wjiang/ijindex_error branch October 8, 2024 19:19
@mathomp4
Copy link
Member

For posterity, I ran GEOS at C270 before and after this fix. This is the output of SO2EMVN before the fix:

stock_volc

and then after the fix:

weiyuan_volc

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants