Replace current_peset_num with 'n' which should be the active pelist. #1246
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jun 6, 2023
|
@laurenchilutti I know @bensonr is away for a couple of weeks. Is it possible someone else could review this change? |
|
@lharris4 I will find someone in the group to review this. |
rem1776
previously approved these changes
Jun 7, 2023
@thomas-robinson will need to review this to merge since he's the other code owner for this file. I just approved as well, but doesn't make a difference merging-wise. |
|
Hi, all. Thanks.
…On Wed, Jun 7, 2023 at 2:57 PM Ryan Mulhall ***@***.***> wrote:
@lharris4 <https://github.com/lharris4> I will find someone in the group
to review this.
@thomas-robinson <https://github.com/thomas-robinson> will need to review
this to merge since he's the other code owner for this file. I just
approved as well, but doesn't make a difference merging-wise.
—
Reply to this email directly, view it on GitHub
<#1246 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMUQRVGTOHNFSXLUYBEYMPLXKDFIDANCNFSM6AAAAAAY3W2JEA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
We will discuss this today at our group meeting |
|
@dkokron how immediately would you like to see this change released in FMS? Would mid July be too late? |
|
Hmm, that delays
NOAA-GFDL/GFDL_atmos_cubed_sphere#272 quite a bit.
I do have an alternative solution that uses MPI directly. Hopefully the
model group will agree to use the MPI solution until FMS is available and
approved for use by NOAA.
Dan
…On Thu, Jun 8, 2023 at 7:28 PM laurenchilutti ***@***.***> wrote:
@dkokron <https://github.com/dkokron> how immediately would you like to
see this change released in FMS? Would mid July be too late?
—
Reply to this email directly, view it on GitHub
<#1246 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACODV2B43FUG3LUDYTRYJIDXKJU3LANCNFSM6AAAAAAY3W2JEA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
thomas-robinson
previously approved these changes
Jun 9, 2023
|
@dkokron We would prefer adding your single fix to a patch instead of using a workaround if the July timeline is too far in the future. We should be able to sync it up with the merge of NOAA-GFDL/GFDL_atmos_cubed_sphere#272 |
dkokron
pushed a commit
to dkokron/GFDL_atmos_cubed_sphere
that referenced
this pull request
Jun 9, 2023
…epends on NOAA-GFDL/FMS#1246 to be functional. More efficient 'if' test in fill_nested_grid_cpl()
dkokron
pushed a commit
to dkokron/fv3atm
that referenced
this pull request
Jun 9, 2023
…epends on NOAA-GFDL/FMS#1246 to be functional. More efficient 'if' test in fill_nested_grid_cpl()
dkokron
pushed a commit
to dkokron/ufs-weather-model
that referenced
this pull request
Jun 9, 2023
…epends on NOAA-GFDL/FMS#1246 to be functional. More efficient 'if' test in fill_nested_grid_cpl()
The base branch was changed.
|
@dkokron Please let us know when you hear back from HAFS regarding the timeline in which you need this change. If you do require it earlier then our next planned release of FMS (which is approx. mid July) we would then need about 1 week to issue a patch for you. |
|
Based on feedback from the HAFS developers, I think it's best to wait for the next scheduled FMS release. |
rem1776
approved these changes
Jun 13, 2023
thomas-robinson
approved these changes
Jun 16, 2023
|
Thanks @dkokron we are going to put it into our next round of testing. |
bensonr
pushed a commit
to NOAA-GFDL/GFDL_atmos_cubed_sphere
that referenced
this pull request
Jan 22, 2024
…rent (#272) * Modifications to replace numerous p2p transfers in fill_nested_grid_cpl by a single MPI_Bcast * Add reason for including ESMF in fv_control_init * Add code to cleanup structures associated with Bcast_comm * Use mpp_broadcast instead of MPI_Bcast at the request of NOAA-GFDL * Get rid of mpp pelist scoping calls in fill_nested_grid_cpl(). This depends on NOAA-GFDL/FMS#1246 to be functional. More efficient 'if' test in fill_nested_grid_cpl() * Bcast_comm and sending_proc variables not needed anymore * New member of the fv_atmos_type type called BcastMember allows more efficient determination of ranks involved in the mpp_broadcast call in fill_nested_grid_cpl routine * Port to FMS-2023.02 --------- Co-authored-by: Dan Kokron <Daniel.Kokron@noaa.gov>
jkbk2004
pushed a commit
to NOAA-EMC/fv3atm
that referenced
this pull request
Jan 22, 2024
…rent (#743) * Modifications to replace numerous p2p transfers in fill_nested_grid_cpl by a single MPI_Bcast * Add reason for including ESMF in fv_control_init. Add code to cleanup structures associated with Bcast_comm * Get rid of unneeded mpp_sync_self * Use mpp_broadcast instead of MPI_Bcast at the request of NOAA-GFDL * Get rid of mpp pelist scoping calls in fill_nested_grid_cpl(). This depends on NOAA-GFDL/FMS#1246 to be functional. More efficient 'if' test in fill_nested_grid_cpl() * Bcast_comm and sending_proc variables not needed anymore * New member of the fv_atmos_type type called BcastMember allows more efficient determination of ranks involved in the mpp_broadcast call in fill_nested_grid_cpl routine * Port to FMS-2023.02
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While chasing down the cause of the following error message, I realized that the check to ensure the transmitting rank is part of the peset is not using the peset returned by the get_peset call.
"FATAL from PE 36: MPP_BROADCAST: broadcasting from invalid PE."
I converted part of the UFS/HAFS application to use mpp_broadcast() instead of numerous point-to-point messages. Testing with a HAFS case revealed the above error message. I was able to make the code work by surrounding the mpp_broadcast() call with appropriate mpp_set_current_pelist() calls. A colleague said these mpp_set_current_pelist() calls should not be needed and provided a unit test to prove it.
Further investigation without the mpp_set_current_pelist() scoping calls revealed the issue addressed with the PR.
Fixes #1243
Testing was performance under Ubuntu-22.04 using the Intel 2023.1 suite of compilers and OpenMPI-4.1.5.
I have run the FMS internal check (make check) with and without the change in this PR. No change in the results.
I have also run "make distcheck". The logs config, make, check and distcheck logs are attached.
checkOPT.log
config.log
distcheckOPT.log
makeOPT.log
I also ran a HAFS case with a single nest on the NOAA 'acorn' system. This is the original case that started this whole investigation. That case ran fine using an FMS with the change contained in his PR.
Checklist:
make distcheckpasses