Create FatesConstantMod to hold global constants #136
Conversation
Create FatesConstantMod to hold global cosntants for things like string length, kind types, spval and other immutable data. Initial values based on similar constants in CLM. Fixes: #135 User interface changes?: none Code review: self Test suite: ed - yellowstone gnu, intel, pgi Test baseline: 49733e8 Test namelist changes: none Test answer changes: bit for bit Test summary: all tests pass Test suite: clm_short - yellowstone gnu, intel, pgi Test baseline: clm4_5_12_r195 Test namelist changes: none Test answer changes: bit for bit Test summary: all tests pass
| public | ||
|
|
||
| ! kinds | ||
| integer, parameter :: fates_r8 = selected_real_kind(12) ! 8 byte real |
There was a problem hiding this comment.
can we strip this down to something shorter like "f_r8" or "fr8"? I feel like this will be used so often that we can reduce total letter count with an abbreviation.
There was a problem hiding this comment.
When you use it, you can still alias it to r8 with something like 'use FatesConstantsMod, only : r8 => fates_r8'. See how it is used in the history module. The actual declaration should have a meaningful, unambiguous name.
There are no bonus points in software engineering for 'reducing the total letter count'. The compiler doesn't care. Code is written once, refactored a few times, and read many, many, many times by human beings. Meaningful names without cryptic abbreviations optimize for the most common use case. In science, it is often humans with little to no training and interest in software as an end. The more clear the code is, the less time it takes to ramp up, and the fewer mistakes they will make. Plus, all modern editors have some form of autocompletion.
There was a problem hiding this comment.
I like the solution of
use FatesConstantsMod, only : r8 => fates_r8
since it only requires changing one line per module...
There was a problem hiding this comment.
Not seeing the alias was an over-site on my part. Try to relax a little bit @bandre-ucar. I have arguments that I can go into that validate why I made the suggestion I did, but I'm not. The way your are proposing to go forward is fine. The important thing is that we create a culture on these forums where people feel welcomed to interact and know that their input will be valued.
| integer, parameter :: fates_long_string_length = 199 | ||
|
|
||
| ! various magic numbers | ||
| real(fates_r8), parameter :: fates_special_value = 1.0e36_fates_r8 ! special value for real data, compatible with clm. |
There was a problem hiding this comment.
We have something similar in clm_fates%init(). A call to set_fates_ctrlparms('hio_ignore_val',rval=spval) transfers the host model's definition of the special value to FATES. It transfers the special value to be used exclusively as a flag to initializing unset history output arrays.
I was thinking it would be nice to have a special section where we house constants that are dictated by the host, I am agnostic as to where that could be located.
There was a problem hiding this comment.
I added spval on a whim because I that was what started this PR, even though I'm not using it. It's not immediately obvious if we need to distinguish between fates_spval and host_spval (seems useful, but maybe not). I'm removing fates specific special values until needed.
The special values added int FatestConstantsMod weren't used at this
time, and it's unclear if we need a fates specific set of special
values, or if we only want to use the host value. Removing until
needed.
Testing: since this only removed unused code, compiled and ran:
SMS_D_Ld5.f45_f45.ICLM45ED.yellowstone_intel.clm-edTest - passed
|
testing on lawrencium |
|
all pass, merging |
Create FatesConstantMod to hold global cosntants for things like
string length, kind types, spval and other immutable data. Initial
values based on similar constants in CLM.
Fixes: #135
User interface changes?: none
Code review: self
Test suite: ed - yellowstone gnu, intel, pgi
Test baseline: 49733e8
Test namelist changes: none
Test answer changes: bit for bit
Test summary: all tests pass
Test suite: clm_short - yellowstone gnu, intel, pgi
Test baseline: clm4_5_12_r195
Test namelist changes: none
Test answer changes: bit for bit
Test summary: all tests pass