search

CICE-Consortium / Icepack

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

icepack_therm_vertical: add new local variables `delt`, `deltq` instead of using `worka`, `workb` #375

Open phil-blain opened 2 years ago

phil-blain commented 2 years ago

While comparing CICE6 with CICE4 in the context of my project, I noticed that the variables worka, workb in subroutine icepack_step_therm1 should probably be renamed to delt, deltq (or at least new variables created with these names).

They are declared here: https://github.com/CICE-Consortium/Icepack/blob/27b872c8882af2521a350633d9a7d088844b6a2f/columnphysics/icepack_therm_vertical.F90#L2397-L2399

And sent to icepack_atm_boundary here:

https://github.com/CICE-Consortium/Icepack/blob/27b872c8882af2521a350633d9a7d088844b6a2f/columnphysics/icepack_therm_vertical.F90#L2705-L2712

where they are received as the delt and delq arguments: https://github.com/CICE-Consortium/Icepack/blob/27b872c8882af2521a350633d9a7d088844b6a2f/columnphysics/icepack_atmo.F90#L855-L862

I notice writing this that worka and workb are also used in other more recent computations.

Anyway, these names date back to the use of global, temporary work arrays in CICE4 and it would be a good clean up to add new local variables with names delt and delq and pass those to icepack_atm_boundary, I think...

I also noticed the following:

eclare108213 commented 2 years ago

Changing the names of local variables is fine with me, if it makes the code easier to follow. Reusing work arrays was originally to minimize memory use but we can hardly claim to be efficient with that anymore.

intent(inout) was used to prevent an extra copy when passing arguments in and out of subroutines, and so was used throughout the code for this small bit of computational efficiency gain. For some reason, intent(out) did an extra copy that (inout) didn't have to do. I'm not sure whether this is still an issue with newer compilers. The tradeoff, of course, is some confusion about whether the variable actually needs to be passed in. I used it as a convenient excuse for gathering initializations into one code block in a higher routine.