Skip to content

Conversation

@philipwjones
Copy link
Contributor

Two ocean modules had local arrays that were not properly sized to accommodate the practice of using nCellsAll+1 (Edges, Vertices) as a location to represent non-existent cells, edges, vertices. This would result in out-of-bounds references. This fix just extends the dimension by one for the arrays that will potentially be referenced in this way.

Fixes #597

[b4b] in QU240 test on Summit - meaning out of bounds refs so far had fortunately no impact

  two ocean modules had local arrays that were
    not properly sized to accommodate the practice
    of using nCellsAll+1 (Edges, Vertices) as a
    location to represent non-existent cells, edges, vertices
  this commit extends the dimension by one for these arrays
Copy link
Collaborator

@mattdturner mattdturner left a comment

Choose a reason for hiding this comment

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

Approved by visual inspection

@mark-petersen mark-petersen self-assigned this Jun 8, 2020
Copy link
Contributor

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

Approved by inspection. I'm surprised this didn't cause problems on other compilers.

  - previous commit changed array sizes in ocn mesh routine but
    did not change following initialization loops.
  - previous commit missed some similar loops in the update routine
@philipwjones
Copy link
Contributor Author

Sorry guys, got a bit sloppy. When I fixed array sizes in the mesh module, I forgot to change subsequent initialization loop limits. Just pushed a fix (well, actually two commits because I missed a second set of loops in the update routine). Still b4b, and actually works in the branch where I triggered this bug.

@mark-petersen
Copy link
Contributor

Tested with gnu and intel, optimized and debug, is bfb with previous.

@mark-petersen mark-petersen merged commit 2fb94eb into MPAS-Dev:ocean/develop Jun 11, 2020
mark-petersen added a commit to E3SM-Project/E3SM that referenced this pull request Jun 11, 2020
fixed bad array sizes in local ocean arrays
  MPAS-Dev/MPAS-Model#598
Remove RediKappa from gm package
  MPAS-Dev/MPAS-Model#602
Fixes bugs in tracer_advection_std
  MPAS-Dev/MPAS-Model#587
jonbob added a commit to E3SM-Project/E3SM that referenced this pull request Jun 15, 2020
…3634)

Update mpas-source: three ocean bug fixes

This PR brings in a new mpas-source submodule with changes only to the
ocean core. It fixes three small bugs that do not impact any current
E3SM test configurations:
* fixes bad array sizes in local ocean arrays (MPAS-Dev/MPAS-Model#598)
* removes RediKappa from gm package (MPAS-Dev/MPAS-Model#602)
* fixes bugs in tracer_advection_std (MPAS-Dev/MPAS-Model#587)

[BFB]
jonbob added a commit to E3SM-Project/E3SM that referenced this pull request Jun 16, 2020
Update mpas-source: three ocean bug fixes

This PR brings in a new mpas-source submodule with changes only to the
ocean core. It fixes three small bugs that do not impact any current
E3SM test configurations:
* fixes bad array sizes in local ocean arrays (MPAS-Dev/MPAS-Model#598)
* removes RediKappa from gm package (MPAS-Dev/MPAS-Model#602)
* fixes bugs in tracer_advection_std (MPAS-Dev/MPAS-Model#587)

[BFB]
@philipwjones philipwjones deleted the ocean/allocfix branch January 8, 2021 20:58
mark-petersen added a commit to mark-petersen/MPAS-Model that referenced this pull request Jan 11, 2021
fixed bad array sizes in local ocean arrays MPAS-Dev#598

Two ocean modules had local arrays that were not properly sized to
accommodate the practice of using nCellsAll+1 (Edges, Vertices) as a
location to represent non-existent cells, edges, vertices. This would
result in out-of-bounds references. This fix just extends the dimension
by one for the arrays that will potentially be referenced in this way.

Fixes MPAS-Dev#597

[b4b] in QU240 test on Summit - meaning out of bounds refs so far had
fortunately no impact
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.

4 participants