Skip to content

Increased the initial hash size for speedup#1208

Open
atrayano wants to merge 1 commit intodevelopfrom
bugfix/inittable_hash
Open

Increased the initial hash size for speedup#1208
atrayano wants to merge 1 commit intodevelopfrom
bugfix/inittable_hash

Conversation

@atrayano
Copy link
Contributor

@atrayano atrayano commented Feb 11, 2026

In raster file processing for tile file generation:

  • Initialized iTable and rTable after allocation.
  • Increase hash size to improve performance.

These changes, in combination with a fix in MAPL GEOS-ESM/MAPL#4381, fix errors in the tile file generation for c5760 (make_bcs).

@atrayano atrayano requested a review from a team as a code owner February 11, 2026 20:29
@atrayano atrayano added the 0 diff The changes in this pull request have verified to be zero-diff with the target branch. label Feb 11, 2026
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.

@atrayano : Many thanks! Do we know that this change fixes make_bcs for c5760? It would be good to add a note in the PR's introductory comment to that effect. It'll be helpful to record the motivation for the PR for later reference.

cc: @biljanaorescanin @weiyuan-jiang

@atrayano
Copy link
Contributor Author

@gmao-rreichle This PR alone does not fix the issues with make_bcs for C5760. We had to separately fix a bug in MAPL (PR #4381) which already has been merged with develop. However, in the process of troubleshooting the bug, we discovered the 2 tables (iTable and rTable) were not properly initialized after memory allocation. We also changed the hash size to improve performance. So, to produce correct results with make_bcs at C5760, you need to use MAPL from develop (until Matt releases a new tag for MAPL). Using this version of CombineRasters.F90 will help performance.

Rasterization of C5760 (quite possibly C2880 and C3072) might need a bigger (finer) raster size ("true" 10 arc-seconds). For example, a grid box near the equator at C5760 is "covered" by only 2x2 raster pixels at our typical 30 arc-second raster 43200x21600). This topic requires a lot of considerations and merits a separate discussion.

@biljanaorescanin
Copy link
Contributor

I've run BCs package for C5760 and C2880 with:

Repo Original Current
MAPL (t) v2.63.1 (DH) (b) develop
GEOSgcm_GridComp (b) develop (b) bugfix/inittable_hash

I can confirmed with this change c2880 and c5760 are zero diff to what we produced for v12 set in the past.

Now, I need tag from @mathomp4 for MAPL + to merge this branch so I can update GEOSldas and we can run for v13 set.

@mathomp4
Copy link
Member

mathomp4 commented Feb 17, 2026

I've run BCs package for C5760 and C2880 with:

Repo Original Current
MAPL (t) v2.63.1 (DH) (b) develop
GEOSgcm_GridComp (b) develop (b) bugfix/inittable_hash
I can confirmed with this change c2880 and c5760 are zero diff to what we produced for v12 set in the past.

Now, I need tag from @mathomp4 for MAPL + to merge this branch so I can update GEOSldas and we can run for v13 set.

Good to hear! As for MAPL, I did release MAPL 2.66.0. I should probably make PRs to GEOSgcm for that.

ETA: I made a PR for MAPL into v11 here: GEOS-ESM/GEOSgcm#1034

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.

4 participants