Extend string interface in fms_string_utils_mod
#1142
Conversation
Replace "sucess" with "success" and "sucessful" with "successful".
Replace "they" with "the".
Use the name "optional_flag" instead of "optional", because the latter is a Fortran keyword.
Extend and improve `string()` in `fms_string_utils_mod`. `r4_kind` and `r8_kind` versions of `string_from_real` have been implemented. Leading and trailing whitespace are now stripped from the returned string. Functions to convert 1D, 2D, and 3D arrays to strings have been implemented and added to the `string` interface.
A unit test for `string()` from `fms_string_utils_mod` has been implemented.
In relation to the extension of the `string` interface in `fms_string_utils_mod`, the new include directory has been added to `CMakeLists.txt` and the new include files have been added to `string_utils/Makefile.am`.
J-Lentz
requested review from
bensonr,
thomas-robinson and
rem1776
as code owners
`ifort` doesn't like spaces between real constants and kind values, so the spaces have been removed.
Use explicit format strings in `string_from_r4()` and `string_from_r8()`. This makes the string conversions consistent across compilers, allowing the unit test to pass on both the GNU and Intel compilers. This change required `string_from_r4` and `string_from_r8` to be moved back from the include file into `fms_string_utils.F90`, since the two functions now use different format strings.
Add a function for Boolean values to the `string` interface. Add three new test cases to the unit test.
Create a `stringify` interface in `fms_string_utils_mod`. Move the real array stringification functions from `string` to `stringify`.
Accept an optional format string as the second argument to `string_from_r4()`, `string_from_r8()`, and `stringify()`. If no format string is supplied, `*` is used.
Increase the length of the temporary string in `string_from_r4`/`string_from_r8` from 25 to 32 characters.
|
|
||
| class default | ||
| call mpp_error(FATAL, "string() called with incompatible argument") | ||
| end select | ||
| end function | ||
|
|
There was a problem hiding this comment.
please put the name of the function here
end function string
There was a problem hiding this comment.
Addressed in 16356ac.
| function string(v, fmt) | ||
| class(*), intent(in) :: v ! Value to be converted to a string | ||
| character(*), intent(in), optional :: fmt !< Optional format string for a real argument | ||
| character(:), allocatable :: string |
There was a problem hiding this comment.
If it's always being allocated to a length of 32, why not just make this a len=32 string instead of allocatable? I'm not saying you have to do this, I'm just wondering if you think that's a better option. That would also make this closer to being a pure/elemental function.
There was a problem hiding this comment.
By making it allocatable, leading and trailing whitespace can be stripped so that the user doesn't need to do trim(string(x)). For me at least, this outweighs the benefit of making the function elemental (although I'm biased by the fact that I've only used string() for generating unit test log messages).
string_utils/fms_string_utils.F90
Outdated
| string = v | ||
|
|
||
| class default | ||
| call mpp_error(FATAL, "string() called with incompatible argument") |
There was a problem hiding this comment.
if this check was not writing to stdout, then this might be able to be an elemental function
string_utils/fms_string_utils.F90
Outdated
| string = "False" | ||
| endif | ||
|
|
||
| type is (integer) |
There was a problem hiding this comment.
can you add integer(i4_kind) and integer(i8_kind)?
There was a problem hiding this comment.
Addressed in 16356ac.
|
|
||
| type is (real(r4_kind)) | ||
| allocate(character(32) :: string) | ||
| if (present(fmt)) then |
There was a problem hiding this comment.
idk if you can check this, but how will you know that the user's format will work?
There was a problem hiding this comment.
I think fmt validation might not be worth the effort, since Fortran will just throw a runtime error if the format string is invalid. The most catastrophic scenario I can imagine would be a format string wider than 32 characters, but the runtime library seems to check this and terminate the program if this happens. It might be a good idea to add a unit test for this case.
string_utils/fms_string_utils.F90
Outdated
|
|
||
| type is (integer) | ||
| allocate(character(32) :: string) | ||
| write(string, '(i0)') v |
There was a problem hiding this comment.
If the user provides a format statement for an integer or a logical or a string, it will be unused. This could produce unexpected behavior for the user.
There was a problem hiding this comment.
Addressed in 16356ac via a warning message (in the case of a logical value) and by using fmt (in the case of an integral value).
string_utils/fms_string_utils.F90
Outdated
| type is (character(*)) | ||
| string = v |
There was a problem hiding this comment.
You should avoid this. The class(*) string handling is inconsistent across compilers and this will only lead to problems.
string_utils/fms_string_utils.F90
Outdated
| string = v | ||
|
|
||
| class default | ||
| call mpp_error(FATAL, "string() called with incompatible argument") |
There was a problem hiding this comment.
If you keep the error message, it should be more comprehensive. "string() called with incompatible argument. Must be of type integer(kind=4), integer(kind=8), real(kind=4), real(kind=8), or logical."
There was a problem hiding this comment.
Addressed in 16356ac.
string_utils/fms_string_utils.F90
Outdated
| !> @brief Converts a number or a Boolean value to a string | ||
| !> @return The argument as a string | ||
| function string(v, fmt) | ||
| class(*), intent(in) :: v ! Value to be converted to a string |
There was a problem hiding this comment.
This variable is not correctly doxygenized
There was a problem hiding this comment.
Addressed in 16356ac.
| class(*), intent(in) :: v ! Value to be converted to a string | ||
| character(*), intent(in), optional :: fmt !< Optional format string for a real argument | ||
| class(*), intent(in) :: v !< Value to be converted to a string | ||
| character(*), intent(in), optional :: fmt !< Optional format string for a real or integral argument |
There was a problem hiding this comment.
There is a typo. Should be "integer" not "integral"
* fix - gnu and pgi issue with class(*) in send_data3d (NOAA-GFDL#1149) * feat: extend `string` interface in `fms_string_utils_mod` (NOAA-GFDL#1142) * fix: add omp directives for race condition in tridiagonal (NOAA-GFDL#1109) * fix: missing if statement in PR NOAA-GFDL#1149 (NOAA-GFDL#1155) * fix: time_manager missing changes from year to yr, month to mo, day to dy (NOAA-GFDL#1169) * chore: update changelog and version numbers for release (NOAA-GFDL#1167) * chore: append dev to version number post-release (NOAA-GFDL#1179) --------- Co-authored-by: Niki Zadeh <niki.zadeh@noaa.gov> Co-authored-by: Jesse Lentz <42011922+J-Lentz@users.noreply.github.com> Co-authored-by: ganganoaa <121043264+ganganoaa@users.noreply.github.com> Co-authored-by: MiKyung Lee <58964324+mlee03@users.noreply.github.com> Co-authored-by: Ryan Mulhall <35538242+rem1776@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Description
The
stringinterface infms_string_utils_modhas been extended to support the following types of input:r4_kindandr8_kindrealsAdditionally, leading and trailing whitespace is now stripped from the output for real arguments.
Fixes #1141
How Has This Been Tested?
A unit test has been implemented. The test passes on the AMD box with
gcc 12.2.0.Checklist:
make distcheckpasses