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

ice_init: do broadcast 'default_season' #766

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

phil-blain
Copy link
Member

@phil-blain phil-blain commented Oct 6, 2022

PR checklist

  • Short (1 sentence) summary of your PR:
    title
  • Developer(s):
    me
  • Suggest PR reviewers from list in the column to the right.
  • Please copy the PR test results link or provide a summary of testing completed below.
    Base suite is b4b with main on robert_intel:
349 measured results of 349 total results
349 of 349 tests PASSED
0 of 349 tests PENDING
0 of 349 tests MISSING data
0 of 349 tests FAILED
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit (since no test in the base suite change default_season)
    • different at roundoff level
    • more substantial if changing default_season
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • No
  • Does this PR update the Icepack submodule? If so, the Icepack submodule must point to a hash on Icepack's main branch.
    • Yes
    • No
  • Does this PR add any new test cases?
    • Yes
    • No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:

When the 'default_season' namelist setting was added in 01494c7 (Nml settings (#208), 2018-10-19) to replace 'l_winter' and 'l_spring', a call to 'broadcast_scalar' was forgotten, such that the 'default_season' value from the namelist is only used on the first MPI process; all other processes get the hardcoded default value 'winter' defined in 'ice_init::input_data', resulting in different initialization across the grid for several variables if anything other than 'winter' is used in the namelist.

Fix that by broadcasting 'default_season' to all MPI procs.

When the 'default_season' namelist setting was added in 01494c7 (Nml
settings (CICE-Consortium#208), 2018-10-19) to replace 'l_winter' and 'l_spring', a
call to 'broadcast_scalar' was forgotten, such that the 'default_season'
value from the namelist is only used on the first MPI process; all other
processes get the hardcoded default value 'winter' defined in
'ice_init::input_data', resulting in different initialization across the
grid for several variables if anything other than 'winter' is used in
the namelist.

Fix that by broadcasting 'default_season' to all MPI procs.
@apcraig
Copy link
Contributor

apcraig commented Oct 6, 2022

This is definitely a bug that could affect users if they set default_season to something other than the internal default. Just didn't affect any of our testing. Maybe we should try to fill that whole in the testing. default_season seems to set initial forcing values. Are they ever used and if so, under what conditions?

@eclare108213 eclare108213 self-assigned this Oct 7, 2022
@eclare108213 eclare108213 self-requested a review October 7, 2022 13:27
@eclare108213
Copy link
Contributor

The default_season 'forcing' serves several purposes -- initialization of potentially missing forcing fields, simple values for short-period testing, and order-of magnitude guidance for users' own forcing fields. Thank you for fixing this bug, @phil-blain! @apcraig, if you want to add a test case for a different season, it could also test starting the code on a date other than Jan 1.

@phil-blain
Copy link
Member Author

I can add a new test to this PR if we want to do that. I noticed that we do have tests with different default_season, but only in the Icepack base suite, not CICE's.

@apcraig apcraig merged commit 578c111 into CICE-Consortium:main Oct 11, 2022
@phil-blain phil-blain deleted the broadcast-default-season branch December 12, 2022 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants