CICE-Consortium / Icepack

Development repository for sea-ice column physics
Other
25 stars 131 forks source link

Index error in bgc tracer for shortwave calculations #408

Closed njeffery closed 1 year ago

njeffery commented 1 year ago

The tracer array is reduced before passing to ice shortwave for chlorophyll radiative calculations, but the shortwave subroutine assumes that the full tracer is present. In MPAS-seaice, the full tracer is passed.

eclare108213 commented 1 year ago

@njeffery Could you provide a few more details? Which tracer array, where it is reduced but shouldn't be, etc. Linking to lines of code in either Icepack or MPAS-SI would be very helpful, or submit a draft PR with code diffs (doesn't need to be ready to merge) in either the Consortium repo or the E3SM Icepack fork, or just tell us what/where to change. Thanks

njeffery commented 1 year ago

@elare108213: The bug only impacts the shortwave scheme when BGC is active and the flag to turn on chlorophyll absorption is on. So, not critical for most configurations. There are two ways to correct the error. Either continue using the subset of trcrn and change the index in the shortwave scheme or pass the full trcrn and keep the index as it is. The latter is what's done in mpas-seaice. Since we're bringing in mpas-seaice column bgc soon, maybe we should pass the full trcrn?

Adrian flagged the difference in tracers passed to the shortwave scheme here:

https://github.com/E3SM-Project/E3SM/blob/master/components/mpas-seaice/src/column/ice_shortwave.F90#L3846 https://github.com/CICE-Consortium/Icepack/blob/main/columnphysics/icepack_shortwave.F90#L3868

https://github.com/E3SM-Project/E3SM/blob/master/components/mpas-seaice/src/column/ice_shortwave.F90#L3869 https://github.com/CICE-Consortium/Icepack/blob/main/columnphysics/icepack_shortwave.F90#L3889

njeffery commented 1 year ago

A similar problem also occurs for the z-aerosols.

https://github.com/E3SM-Project/E3SM/blob/master/components/mpas-seaice/src/column/ice_shortwave.F90#L3886 https://github.com/CICE-Consortium/Icepack/blob/main/columnphysics/icepack_shortwave.F90#L3912

eclare108213 commented 1 year ago

If I understand correctly, Icepack (and CICE) send in and use a slice of the full tracer array, while MPAS-SI sends in and uses the entire tracer array. Therefore the indexing in the radiation routines is different in the two codes but consistent in both places. Is that correct? If so, then this is not necessarily a bug in the current Icepack code, but rather something we will fix (one way or the other) during the merge process. Do you agree?

MPAS-SI: trtmp0(nt_bgc_N(1) + k-1) = trtmp0(nt_bgc_N(1) + k-1) + R_chl2N(n)*F_abs_chl(n)*trcrn(nt_bgc_N(n)+k-1)

Icepack: trtmp0(nt_bgc_N(1) + k-1) = trtmp0(nt_bgc_N(1) + k-1) + R_chl2N(n)*F_abs_chl(n)*bgcN(nt_bgc_N(n)-nt_bgc_N(1)+k)

njeffery commented 1 year ago

@eclare108213: Yes, you're right! I looked too quickly yesterday and mistakenly thought the indexing was the same in the two codes. You can close this issue.

eclare108213 commented 1 year ago

Thanks for pointing it out -- we will need to decide how to handle the tracer arrays as they are passed through the interface.