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

Changes to return correct dimensions for the MPI grid from the OpenMP subgrids #3350

Merged
merged 7 commits into from
Jan 24, 2025

Conversation

aoloso
Copy link
Contributor

@aoloso aoloso commented Jan 17, 2025

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)
  • Refactor (no functional changes, no api changes)

Checklist

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

Description

Changes were made to add attributes to the subgrids (i.e. created by dividing the MPI subdomain into smaller subdomains equal to the number of OpenMP threads) such that the correct dimensions for the MPI subdomain could be retrieved from the subgrids where ever needed.

Related Issue

use_threads set to True for openMp Issue #3274

@aoloso aoloso added the 0 Diff The changes in this pull request have verified to be zero-diff with the target branch. label Jan 17, 2025
@aoloso aoloso requested a review from a team as a code owner January 17, 2025 23:52
@weiyuan-jiang
Copy link
Contributor

I can confirm this fix works on Hercules with openmp

weiyuan-jiang
weiyuan-jiang previously approved these changes Jan 21, 2025
@mathomp4
Copy link
Member

@aoloso Can you add a changelog entry for this?

@mathomp4
Copy link
Member

Note: I did try this out with a simple AMIP run (so not exercising it probably) and it was zero-diff. So it doesn't seem to affect our way of running

tclune
tclune previously approved these changes Jan 22, 2025
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.

How is the test coming along? It will be more difficult to verify the test once the fix is merged. You'll have to checkout a pre-fixed version but that version also won't have the test ...

@mathomp4 mathomp4 dismissed stale reviews from tclune and weiyuan-jiang via 260d563 January 23, 2025 18:35
@mathomp4 mathomp4 merged commit 6ad5003 into develop Jan 24, 2025
48 checks passed
@mathomp4 mathomp4 deleted the fix_with_ben branch January 24, 2025 15:41
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.

6 participants