Small answer changes in preparation for adding option for bio-mass heat storage #1241
Conversation
…t_veg, dlrad and ulrad, this is shown to be different on the PGI compiler, but should be only different by roundoff, since it's just a difference in order of operations for those terms
…d another additional test as well for one of the new compsets
… e-11 and e-12, e-13 showed problems
ekluzek
added
enhancement
src/biogeophys/CanopyFluxesMod.F90
Outdated
| if ( abs(temp - dt_veg(p) ) > 1.e-12 )then | ||
| call endrun( "Difference greater than roundoff" ) | ||
| end if |
There was a problem hiding this comment.
@ekluzek - Thanks for all of your work to demonstrate only roundoff-level diffs. I know from my own experience how painful that can sometimes be.
One question: are the terms you're checking here all order 1 (in magnitude)? If not, or if you're not sure, I generally use relative diff checks for things like this:
if (abs(temp - dt_veg(p)) > 1.e-13_r8 * temp) thenThere was a problem hiding this comment.
That's a good point. Actually I'm sure they aren't order 1. dt_veg and err will be order 1, but could also be very small. ulrad and dlrad should be order 100. So doing a relative diff would be a better comparison. If I do that I'm likely to be able to show a relative difference that's a bit smaller as well.
There was a problem hiding this comment.
No need to redo things, though, if you have already done it this way and you have a general sense of the magnitude. My main worry with abs diffs is if the terms are very small to begin with - so an abs diff of 1e-12 could be a relative diff of, say, 1e-6. If you're pretty confident that isn't generally the case here, then I think this is good enough.
There was a problem hiding this comment.
I went ahead and did a little testing with this to show it's fine. Relative diff's potentially a bit more troublesome if temp is identically zero, or negative. But with a relative diff I was able to show a tolerance of e-14 was fine for two tests.
There was a problem hiding this comment.
Relative diff's potentially a bit more troublesome if temp is identically zero, or negative.
Just a thought for the future: If temp is the old (presumed correct) version, then I usually have success with something like if (abs(temp - dt_veg(p)) > 1.e-13_r8 * temp) – or change the last temp to abs(temp) if negative values are acceptable. This avoids divide by 0. If temp (the old version) was exactly 0 at a point, then I typically expect the new version to be exactly zero, too, and I generally want to ensure that's true (which is done by the above) – since exactly 0 vs. slightly different from 0 can sometimes lead to large downstream differences.
…n two tests to verify: SMS.f10_f10_musgs.I2000Clm50BgcCrop.izumi_pgi.clm-crop and SMS_Ld1_Mmpi-serial.f45_f45_mg37.I2000Clm50Sp.izumi_pgi.clm-ptsRLA
…ference of order roundoff
|
I think this is complete now. I'm running final testing and will tag once that's complete. |
Description of changes
Some small changes that can change answers in preparation of bringing in the option for bio-mass heat storage. This only changes answers for some compilers. This brings in some new terms (currently set to zero) that will be in use for clm5_1 when bio-mass heat storage is turned on. The terms that change are identical to the BHS version with the exception that some variables have a "p" index in the full BHS version as they are array terms rather than the scalars they are in here.
The full BHS version is PR #1016
Specific notes
Contributors other than yourself, if any: @swensosc
CTSM Issues Fixed (include github issue #): None
Are answers expected to change (and if so in what way)? Roundoff changes for some compilers
Any User Interface Changes (namelist or namelist defaults changes)?
Some new compsets for clm5.1
Change one clm5.0 test to use the new clm5.1 compset, and add another test
Testing performed, if any: Limited testing on izumi
I also added an explicit testing inside the code in an earlier version to ensure the changes were on the order of e-11/e-12 for the terms that change.