Skip to content

Fixes indexing issue in SurfaceAreaWeighted AM #173

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.

Sign up for GitHub

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

Merged
mark-petersen merged 1 commit into from
Mar 21, 2019
Merged

Fixes indexing issue in SurfaceAreaWeighted AM #173

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/core_ocean/analysis_members/mpas_ocn_surface_area_weighted_averages.F
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,10 @@ subroutine ocn_compute_surface_area_weighted_averages(domain, timeLevel, err)!{{
! copy data into work array
! Note: Order of indices must match the ordering of vars in var_arrays in Registry_surface_area_weighted_averages.xml
! (and all var_arrays in Registry_surface_area_weighted_averages.xml must have same ordering!)

!initialize workArray to zeros
workArray(:,:) = 0.0_RKIND
n = 1
workArray(n,:) = 0.0_RKIND

@milenaveneziani milenaveneziani Mar 5, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch @vanroekel. But didn't the n=1 on the previous line help initialize workArray(n,:) correctly?

@vanroekel vanroekel Mar 5, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it only initialized the 1st index, previously all indices were initialized to zero. I've reverted now to using

workArray(:,:) = 0.0_RKIND

which should initialize the whole array to zero and not just workArray(1,:)

@milenaveneziani milenaveneziani Mar 5, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see. I don't even remember when this analysis member was changed. Anyways, thanks for finding this.

n = n + 1

@mark-petersen mark-petersen Mar 5, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You are removing an index advancement here. Were all the variables off by an index of one before?
For example, latentHeatFlux was put in workArray(4,:) and will now be put in workArray(3,:). Do we need to make a corresponding index adjustment somewhere else?

@xylar xylar Mar 5, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, everything is off by one index. Currently, we only use SST in MPAS analysis so that's the only one we noticed.

@mark-petersen mark-petersen Mar 5, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see. I've answered my own question. This is the original in 0b984bd

src/core_ocean/analysis_members/mpas_ocn_surface_area_weighted_averages.F
345          ! copy data into work array
346          workArray( :,:) = 0.0_RKIND
347          workArray( 1,:) = workMask(:)
348          workArray( 2,:) = areaCell(:)
349          if (activeTracersBulkRestoringPKG) workArray( 3,:) = latentHeatFlux(:)

and the bug was introduced in 4b64b2d. So this PR just restores the indexing back to the original.

workArray(n,:) = workMask(:)
n = n + 1
workArray(n,:) = areaCell(:)
Expand Down