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

Adds a broadcast of the namelist value 'urbantvmapalgo' #2534

Open
wants to merge 2 commits into
base: b4b-dev
Choose a base branch
from

Conversation

briandobbins
Copy link
Contributor

Adds a broadcast of the namelist value (urbantvmapalgo) after reading it in so it is consistent across all ranks when not using the default value ('nn').

Description of changes

Simple one-line change; a namelist value wasn't being shared with ranks other than the main one reading the namelist. This lead to hangs when trying a different algorithm.

Specific notes

Contributors other than yourself, if any:

CTSM Issues Fixed (include github issue #):
#2533

Are answers expected to change (and if so in what way)?
No

Any User Interface Changes (namelist or namelist defaults changes)?
No

Does this create a need to change or add documentation? Did you do so?
No

Testing performed, if any:
Limited testing, but it's a one-line logical change. If more is needed, let me know. I just did a simple test with and without the change, looking at the values on all ranks.

… it in so it is consistent across all ranks when not using the default value ('nn').
@briandobbins briandobbins self-assigned this May 9, 2024
Copy link
Member

@billsacks billsacks left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @briandobbins !

@ekluzek
Copy link
Collaborator

ekluzek commented May 16, 2024

@briandobbins I'm rebasing this to come in on b4b-dev, so it can come in quicker.

@ekluzek ekluzek changed the base branch from master to b4b-dev May 16, 2024 15:06
@ekluzek ekluzek added enhancement new capability or improved behavior of existing capability tag: simple bfb labels May 16, 2024
@samsrabin samsrabin added simple bfb bit-for-bit and removed simple bfb labels Aug 8, 2024
@wwieder
Copy link
Contributor

wwieder commented Sep 25, 2024

Is this ready to merge?

@samsrabin
Copy link
Collaborator

samsrabin commented Sep 26, 2024

@briandobbins Would you mind rebasing this onto ctsm5.3.0 and force-pushing? Not a big deal, but makes it clearer exactly what files are being changed.

I tested git rebase ctsm5.3.0 fix_urbantvmapalgo_broadcast and it worked with no conflicts.

@samsrabin samsrabin added the bug something is working incorrectly label Sep 26, 2024
@samsrabin samsrabin assigned samsrabin and unassigned briandobbins Sep 30, 2024
@samsrabin samsrabin added good first issue simple; good for first-time contributors and removed simple labels Oct 3, 2024
@samsrabin
Copy link
Collaborator

@briandobbins Rebase no longer needed; I'll start testing now.

@briandobbins
Copy link
Contributor Author

Argh, my apologies -- I've just been swamped with deadlines. Before you test, though, there's an additional change needed (to enable 'redist' as an option in the namelist_definition_ctsm.xml file for the urbantvmapalgo option). Happy to add this after tonight, so if you want to delay one more day, or do it yourself, that'd be fine.

Thanks so much!

@samsrabin
Copy link
Collaborator

Ah okay, I'll let you handle that. Once you push I'll kick off the testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bfb bit-for-bit bug something is working incorrectly enhancement new capability or improved behavior of existing capability good first issue simple; good for first-time contributors PR status: needs testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants