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

revisit underflow trapping approach and exp_argmax values #170

Open
apcraig opened this issue Feb 13, 2018 · 2 comments
Open

revisit underflow trapping approach and exp_argmax values #170

apcraig opened this issue Feb 13, 2018 · 2 comments

Comments

@apcraig
Copy link
Contributor

apcraig commented Feb 13, 2018

Currently, the underflow trapping is not entirely consistent, which maybe is fine. But it was done somewhat adhoc. Review and modify if appropriate. For instance, maybe we should compute ln(tiny) to define the exp_argmax instead of choosing it arbitrarily. Or maybe exp_argmax should be more uniform across the code. Or maybe there are other approaches.

@eclare108213
Copy link
Contributor

Try uncommenting lines 563, 564 in icedrv_step.F90. This could fix a number of problems, e.g. issues #48 , #167 .
Underflows also appear in the CICE dynamics subroutine stress; this issue is bigger than Icepack. There are different kinds of underflows: those that occur because there isn't any ice (we should mask), those that occur because of coding choices (recode), and those that are produced by a parameterization for physical reasons. The latter comes in different flavors, e.g. radiation attentuation and internal stresses, and will need to be taken on a case-by-case basis. But we could definitely make the trapping more consistent within these types.

@eclare108213
Copy link
Contributor

I search the code (cicecore and icepack) for underflow instances, and did not find others besides the ones in evp and eap and the ones that were discussed and/or fixed here:

exponential underflows in dEdd shortwave #118
icepack_algae underflow #126
fix shortwave underflows and mechred divide by zero #127 PR
ice_brine exception #129
fix more icepack debug errors #132 PR
fix underflow in brine #158 PR
fix mechred exp underflows #169 PR

I also looked at all of the places where puny (10^-11) is used. These are mostly to prevent divide-by-zero, for masking purposes, or for certain physical states (e.g. snow depth disappears during melting, or the sun being below the horizon). These all seem fine to me.

I propose the following:

  1. Uncomment lines in icedrv_step.F90 as mentioned above. This should prevent the code from trying to ridge ice in grid cells where there is no ice, and then we can remove the kludgey fix in ice_mechred (fix mechred exp underflows #169).
  2. For exponentials, set exp_argmax so that exp(-exp_argmax) = puny, everwhere that this is used, and implement it using the 'min' function rather than conditionals (icepack_brine.F90 uses conditionals; other modules are icepack_algae.F90, icepack_mechred.F90, icepack_shortwave.F90).
  3. (This is for CICE/cicecore, not Icepack) Uncomment the lines in ice_dyn_eap.F90 and ice_dyn_evp.F90 that manually prevent underflows of the stresses, to see what the performance hit is now for compilers that do not have a flush-to-zero (ftz) flag. We could also test other compilers by turning off the ftz flag. We could ask our DMI colleagues who are optimizing this routine whether they have better approaches to do this.

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

No branches or pull requests

2 participants