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

Fix 1x1 and 2x2 blocksize error #618

Merged
merged 1 commit into from
Jul 16, 2021
Merged

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Jul 16, 2021

PR checklist

  • Short (1 sentence) summary of your PR:
    Fix 1x1 and 2x2 blocksize error, update initialization of ARRAY_G in global_gather_ext

  • Developer(s):
    apcraig

  • 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.
    Full test suite run on cheyenne with gnu. All results are bit-for-bit except 1x1 and 2x2 results which don't change answers but do change restart files slightly due to the fix in missing blocks. See cheyenne gnu results https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#85531cf8f8ae93f628344df6ecfa4be12e9629d8

  • How much do the PR code changes differ from the unmodified code?

    • bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?

    • 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:

  • Fix 1x1 and 2x2 non-bit-for-bit results. It turns out the 1x1 and 2x2 blocks were
    bit-for-bit physically, just not technically. The problem was that a set
    of blocks were "land block eliminated"
    near the equator incorrectly. These blocks never have sea ice, so it's not a problem
    scientifically, but it changes the masking and restart files. The fix is a one line change
    in ice_domain.F90 which that sets the minimum "flat" value to 1. At the equator, flat
    was set to zero and this eliminated those gridcells incorrectly. This error could only
    happen in cases where the gridcells were within 0.5 degrees of the equator, and a set of
    conditions were required where an active gridcell was set to inactive which then resulted
    in the entire block being eliminated. This could really only happen with the smallest blocks
    which is why it only showed up with 1x1 and 2x2.

  • Update the initialization of ARRAY_G in the gather_global_ext interfaces. Get rid of
    older, more complex approach which left some blocks uninitialized.

  • Set psalt(n) to zero by default. Was uninitialized in cases where vice=0.

  • Add a test to check bit-for-bit in log files for 1x1 blocks with bfbflag set to reprosum.

- Fix 1x1 and 2x2 non-bit-for-bit results.  It turns out the 1x1 and 2x2 blocks were
  bit-for-bit physicallly.  The problem was that a set of blocks were "land block eliminated"
  near the equator incorrectly.  These blocks never have sea ice form, so it's not a problem
  scientifically, but it changes the masking and restart files.  The fix is a one line change
  in ice_domain.F90 which that sets the minimum "flat" value to 1.  This error could only
  happen in cases where the blocks were within 0.5 degrees of the equator, and a set of
  conditions were required where an active gridcell was set to inactive which then resulted
  in the entire block being eliminated.
- Update the initialization of ARRAY_G in the gather_global_ext interfaces.  Get rid of
  older, more complex approach which left some blocks uninitialized.
- Set psalt(n) to zero by default.  Was uninitialized in cases where vice=0.
- Add a test to check bit-for-bit in log files for 1x1 blocks with bfbflag set to reprosum.
@apcraig
Copy link
Contributor Author

apcraig commented Jul 16, 2021

I think this can close #595. This addresses the 1x1 and 2x2 block issue as well as the ARRAY_G issue. The evp1d solver is being addressed in #568.

It would be nice to merge this by Friday afternoon if we can so it can be included in weekend testing.

@apcraig apcraig linked an issue Jul 16, 2021 that may be closed by this pull request
Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

cool

Copy link
Member

@phil-blain phil-blain left a comment

Choose a reason for hiding this comment

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

I'm not that familiar with this part of the code, especially the MPI gather-scatter stuff, but removing a bunch of lines and replacing it with a single line is always welcome!

Thanks.

@apcraig apcraig merged commit 36799ec into CICE-Consortium:master Jul 16, 2021
@apcraig apcraig deleted the tinyblocks branch August 17, 2022 20: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.

bug in gather_global_ext in ice_gather_scatter.F90
3 participants