ESCOMP / ESCOMP/POP2-CESM

Better loop order in ecosys_driver

Open
#12 0 comments 0 reactions 1 assignee Claimed by @mnlevy1981 View on GitHub
Dominant language
Fortran
Stars
8
Forks
27
PR merge metrics
No merged PRs in 30d

Description

### Description of the issue:

When copying data in and out of the MARBL instance prior to calling `surface_flux_compute()`, the loops are not ordered in an efficient manner for memory management. The existing code is

```
!-----------------------------------------------------------------------
! Copy data from slab data structure to column input for marbl
!-----------------------------------------------------------------------

do index_marbl = 1, marbl_col_cnt(iblock)
i = marbl_col_to_pop_i(index_marbl,iblock)
j = marbl_col_to_pop_j(index_marbl,iblock)

do n = 1,size(surface_flux_forcings)
marbl_instances(iblock)%surface_flux_forcings(n)%field_0d(index_marbl) = &
surface_flux_forcings(n)%field_0d(i,j,iblock)
end do

do n = 1,ecosys_tracer_cnt
marbl_instances(iblock)%tracers_at_surface(index_marbl,n) = &
p5*(tracers_at_surface_old(i,j,n) + tracers_at_surface_cur(i,j,n))
end do

do n=1,size(surface_flux_saved_state)
marbl_instances(iblock)%surface_flux_saved_state%state(n)%field_2d(index_marbl) = &
surface_flux_saved_state(n)%field_2d(i,j,iblock)
end do

end do
```

but we would be better served having the `n` loops outside of the `(i,j)` loop. This came up because @klindsay28 and I were looking at the the `tracers_at_surface` loop

```
do n = 1,ecosys_tracer_cnt
marbl_instances(iblock)%tracers_at_surface(index_marbl,n) = &
p5*(tracers_at_surface_old(i,j,n) + tracers_at_surface_cur(i,j,n))
end do
```

and talking about how we don't actually need the surface values of every tracer... so if MARBL could tell POP which tracer indices it actually cares about, we could have an `if (nth tracer not required for surface flux computation) cycle` line (and it would be much better to have that if statement outside the `(i,j)` loop)

Note that this is not applicable to `interior_tendency_compute()` because MARBL needs to make that call column-by-column (and also there is a transpose happening when we copy data into the MARBL structure), but it's something to keep in mind when we introduce [`marbl_instance%reset()`](https://github.com/marbl-ecosys/MARBL/issues/85#issuecomment-509831291)

### Version:
- CESM: latest (2_2_beta02)
- POP2: latest ( f943f01)

### Machine/Environment Description:

N/A

### Any xml/namelist changes or SourceMods:

N/A

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.