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

Halo Update Fill Implemention Bug with Padded Blocks #572

Closed
apcraig opened this issue Mar 9, 2021 · 1 comment · Fixed by #579
Closed

Halo Update Fill Implemention Bug with Padded Blocks #572

apcraig opened this issue Mar 9, 2021 · 1 comment · Fixed by #579

Comments

@apcraig
Copy link
Contributor

apcraig commented Mar 9, 2021

In ice_boundary.F90 (both mpi and serial), the fill implementation in the halo update is carried out assuming blocks are NOT padded. This is a bug. The current implementation looks like

    do j = 1,nghost
       array(1:nx_block,           j,:) = fill
       array(1:nx_block,ny_block-j+1,:) = fill
    enddo
    do i = 1,nghost
       array(i,           1:ny_block,:) = fill
       array(nx_block-i+1,1:ny_block,:) = fill
    enddo

but it should look more like

       do j = 1,nghost
          array(1:nx_block, jlo-j,iblk) = fill
          array(1:nx_block, jhi+j,iblk) = fill
       enddo
       do i = 1,nghost
          array(ilo-i, 1:ny_block,iblk) = fill
          array(ihi+i, 1:ny_block,iblk) = fill
       enddo

One particular challenge is that ilo/ihi/jlo/jhi do not seem to be accessible in ice_boundary due to circular logic, so some data may need to be moved around in the code.

The good thing is that this doesn't seem to affect any results. This was detected by initializing all arrays with signalling nans and then trapping. Without that, you seem to fortuitously get a 0 multiplied by the uninitialized values in all cases. The fact that padded and non-padded decomps produce bit-for-bit results suggests this has not affected answers in actual model runs.

In the one place where the signalling nans was trapped (to_ugrid in ice_grid.F90), we have worked around it by initializing tarea to zero everywhere. When the halo fill is fixed, that fix should be removed, and we should confirm that the original issue is resolved.

See also #560.

@phil-blain
Copy link
Member

For easy reference, one such loop is at:

do j = 1,nghost
array(1:nx_block, j,:) = fill
array(1:nx_block,ny_block-j+1,:) = fill
enddo
do i = 1,nghost
array(i, 1:ny_block,:) = fill
array(nx_block-i+1,1:ny_block,:) = fill
enddo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants