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.

Sign up for GitHub

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

Jump to bottom

Improved mask sampler #2890

Merged
merged 9 commits into from
Jun 26, 2024
Merged

Improved mask sampler #2890

merged 9 commits into from
Jun 26, 2024

Conversation

metdyn
Copy link
Contributor

@metdyn metdyn commented Jun 26, 2024 •
edited
Loading

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

In the Mask sampler (input = OR_ABI-L1b-RadF-M6C04_G16_*.nc), the following improvement is added:

  • parallelization for a do loop over 29.4 million points [nx=ny=5424]
  • add LS_chunk (uniform distributed points) as intermediate step between LS_root and LS_distributed(bkg=CS-grid) to circumvent a slow down from ESMF_redist between root and distributed case.

Time improvement on 126 cores (C360_L137)
HIST control: 49.6 s
new code: 25 s

Now the new code can run with thin_factor = 1; control code will crash with 126 cores.

Related Issue

metdyn added 7 commits June 21, 2024 19:25
@metdyn
begin improve_mask_xygrid
bbefee3
@metdyn
Replace LS_root --> LS_ds with
8a6c11c 
        LS_root -->  LS_chunk --> LS_ds
Get coord. of LS_ds(bk=CS) does not work directly:
        call ESMF_LocStreamGetKey( LS_ds, "ESMF:Lon", farray=ptA) gives wrong results
@metdyn
removed the ESMF_LocStreamGetKey, not working for LS_ds(CS-grid)
62c8e3e
@metdyn
1. Use MPI to process ABI_grid data to Lon/Lat, which is more efficient
4e4562d 
2. add LS_chunk as middle step to get coordinates for LS_ds(CS-grid)
@metdyn
clean up code
74466ce
@metdyn
add change log
11eec59
@metdyn
Merge branch 'develop' into feature/ygyu/improve_mask_xygrid
b1f7714
@metdyn metdyn added the 0 Diff Trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) label Jun 26, 2024
@metdyn metdyn requested a review from a team as a code owner June 26, 2024 02:24
@mathomp4
Copy link
Member

mathomp4 commented Jun 26, 2024 •
edited
Loading

@metdyn I think you meant to make this against develop. Can you change the base of the PR?

Oh, and move your changelog entry up to the Unreleased section as well.

@mathomp4
Copy link
Member

Oh wait. Is this a hotfix? I see in your comment:

Now the new code can run with thin_factor = 1; control code will crash with 126 cores.

So, was there a bug in v2.47.0?

tclune
tclune previously approved these changes Jun 26, 2024
View reviewed changes
@metdyn metdyn changed the base branch from main to develop June 26, 2024 16:10
@metdyn metdyn dismissed tclune’s stale review June 26, 2024 16:10

The base branch was changed.

@metdyn metdyn added 0 Diff The changes in this pull request have verified to be zero-diff with the target branch. and removed 0 Diff Trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) labels Jun 26, 2024
@mathomp4 mathomp4 merged commit cf02db6 into develop Jun 26, 2024
36 checks passed
@mathomp4 mathomp4 deleted the feature/ygyu/improve_mask_xygrid branch August 23, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mathomp4 mathomp4 mathomp4 approved these changes

@tclune tclune tclune left review comments

Assignees
No one assigned
Labels
0 Diff The changes in this pull request have verified to be zero-diff with the target branch.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@metdyn @mathomp4 @tclune