Skip to content

Commit

Permalink
Merge pull request #3258 from GEOS-ESM/bugfix/wdboggs/udsystem_bug_3253
Browse files Browse the repository at this point in the history
Fix udsystem bug with macros and return codes
  • Loading branch information
darianboggs authored Dec 19, 2024
2 parents 0e0a5fa + b1d7b02 commit e1f18d4
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 43 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Fixed bug with return codes and macros in udunits2f

### Removed

### Deprecated
Expand Down
4 changes: 4 additions & 0 deletions udunits2f/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@ list (APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")

esma_add_library(${this}
SRCS ${srcs}
DEPENDENCIES MAPL.shared
TYPE SHARED
)

target_include_directories (${this} PUBLIC
$<BUILD_INTERFACE:${MAPL_SOURCE_DIR}/include>)

find_package(udunits REQUIRED)
find_package(EXPAT REQUIRED)

Expand Down
57 changes: 29 additions & 28 deletions udunits2f/UDSystem.F90
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
#include "error_handling.h"
#include "MAPL_ErrLog.h"
#include "MAPL_Exceptions.h"

module ud2f_UDSystem
use ud2f_CptrWrapper
use ud2f_interfaces
use ud2f_encoding
use ud2f_status_codes
use MAPL_ExceptionHandling
use iso_c_binding, only: c_ptr, c_associated, c_null_ptr, c_null_char
use iso_c_binding, only: c_char, c_int, c_float, c_double, c_loc
implicit none
Expand Down Expand Up @@ -94,13 +96,13 @@ module ud2f_UDSystem

contains

! Check the status for the last udunits2 call
logical function success(utstatus)
! Convert utstatus (UDUNITS2 status) to logical
logical function is_ut_success(utstatus)
integer(ut_status) :: utstatus

success = (utstatus == UT_SUCCESS)
is_ut_success = (utstatus == UT_SUCCESS)

end function success
end function is_ut_success

function construct_system(path, encoding) result(instance)
type(UDsystem) :: instance
Expand All @@ -112,7 +114,7 @@ function construct_system(path, encoding) result(instance)
! Read in unit system from path
call read_xml(path, utsystem, status)

if(success(status)) then
if(is_ut_success(status)) then
call instance%set_cptr(utsystem)
if(present(encoding)) instance%encoding = encoding
return
Expand All @@ -135,7 +137,7 @@ function construct_unit(identifier) result(instance)
cchar_identifier = cstring(identifier)
utunit1 = ut_parse(SYSTEM_INSTANCE%get_cptr(), cchar_identifier, SYSTEM_INSTANCE%encoding)

if(success(ut_get_status())) then
if(is_ut_success(ut_get_status())) then
call instance%set_cptr(utunit1)
else
! Free memory in the case of failure
Expand All @@ -149,15 +151,15 @@ function construct_converter(from_unit, to_unit) result(conv)
type(UDUnit), intent(in) :: from_unit
type(UDUnit), intent(in) :: to_unit
type(c_ptr) :: cvconverter1
logical :: convertible
! logical :: convertible !wdb fixme deleteme

! Must supply units that are initialized and convertible
if(from_unit%is_free() .or. to_unit%is_free()) return
if(.not. are_convertible(from_unit, to_unit)) return

cvconverter1 = ut_get_converter(from_unit%get_cptr(), to_unit%get_cptr())

if(success(ut_get_status())) then
if(is_ut_success(ut_get_status())) then
call conv%set_cptr(cvconverter1)
else
! Free memory in the case of failure
Expand All @@ -174,9 +176,9 @@ subroutine get_converter(conv, from, to, rc)
integer(ut_status) :: status

conv = get_converter_function(from, to)
_ASSERT(.not. conv%is_free(), UTF_CONVERTER_NOT_INITIALIZED)

_RETURN(UT_SUCCESS)
_ASSERT_RC(.not. conv%is_free(), 'Failed to get converter function', UTF_CONVERTER_NOT_INITIALIZED)
_RETURN(_SUCCESS)
_UNUSED_DUMMY(status) !wdb fixme deleteme add blank line after
end subroutine get_converter

! Get converter object
Expand Down Expand Up @@ -315,21 +317,20 @@ subroutine initialize(path, encoding, rc)
integer, optional, intent(out) :: rc
integer :: status

_RETURN_UNLESS(instance_is_uninitialized())
! System must be once and only once.
_ASSERT(instance_is_uninitialized(), UTF_DUPLICATE_INITIALIZATION)
! System must be initialized once and only once.
_ASSERT_RC(instance_is_uninitialized(), 'UDSystem is initialized already.', UTF_INITIALIZATION_FAILURE)

! Disable error messages from udunits2
call disable_ut_error_message_handler()

call initialize_system(SYSTEM_INSTANCE, path, encoding, rc=status)
if(status /= UT_SUCCESS) then
if(.not. is_ut_success(status)) then
! On failure, free memory
call finalize()
_RETURN(UTF_INITIALIZATION_FAILURE)
_RETURN(_FAILURE)
end if
_ASSERT(.not. SYSTEM_INSTANCE%is_free(), UTF_NOT_INITIALIZED)
_RETURN(UT_SUCCESS)
_ASSERT_RC(.not. SYSTEM_INSTANCE%is_free(), 'Failed to initialize UDSystem', UTF_NOT_INITIALIZED)
_RETURN(_SUCCESS)

end subroutine initialize

Expand All @@ -339,13 +340,13 @@ subroutine initialize_system(system, path, encoding, rc)
integer(ut_encoding), optional, intent(in) :: encoding
integer, optional, intent(out) :: rc
integer :: status
type(c_ptr) :: utsystem
! type(c_ptr) :: utsystem !wdb fixme deleteme

! A system can be initialized only once.
_ASSERT(system%is_free(), UTF_DUPLICATE_INITIALIZATION)

_ASSERT_RC(system%is_free(), 'UDSystem is initialized already.', UTF_INITIALIZATION_FAILURE)
system = UDSystem(path, encoding)
_RETURN(UT_SUCCESS)
_RETURN(_SUCCESS)
_UNUSED_DUMMY(status)
end subroutine initialize_system

! Is the instance of the unit system initialized?
Expand Down Expand Up @@ -376,7 +377,7 @@ end subroutine free_ut_unit
! Free memory for converter
subroutine free_cv_converter(this)
class(Converter), intent(in) :: this
type(c_ptr) :: cvconverter1
! type(c_ptr) :: cvconverter1 !wdb fixme deleteme

if(this%is_free()) return
call cv_free(this%get_cptr())
Expand All @@ -401,9 +402,9 @@ function are_convertible_udunit(unit1, unit2, rc) result(convertible)

convertible = (ut_are_convertible(unit1%get_cptr(), unit2%get_cptr()) /= ZERO)
status = ut_get_status()
_ASSERT(success(status), status)
_ASSERT_RC(is_ut_success(status), 'Unable to check are_convertible', status)

_RETURN(UT_SUCCESS)
_RETURN(_SUCCESS)
end function are_convertible_udunit

! Check if units are convertible
Expand All @@ -417,9 +418,9 @@ function are_convertible_str(from, to, rc) result(convertible)

unit1 = UDUnit(from)
unit2 = UDUnit(to)
convertible = are_convertible_udunit(unit1, unit2, _RC)
convertible = are_convertible(unit1, unit2, _RC)

_RETURN(UT_SUCCESS)
_RETURN(_SUCCESS)
end function are_convertible_str

! Create C string from Fortran string
Expand Down
21 changes: 11 additions & 10 deletions udunits2f/tests/Test_UDSystem.pf
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "MAPL_ErrLog.h"
module Test_UDsystem

use funit
Expand All @@ -21,9 +22,9 @@ contains
integer(ut_status) :: status

call initialize_udunits_system(rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to initialize')
@assertEqual(_SUCCESS, status, 'Failed to initialize')
call get_converter(conv, KM, M, rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to get converter')
@assertEqual(_SUCCESS, status, 'Failed to get converter')
@assertFalse(conv%is_free(), 'cv_converter is not set')
cptr = conv%get_cptr()
@assertTrue(c_associated(cptr), 'c_ptr is not associated')
Expand All @@ -44,9 +45,9 @@ contains
character(len=*), parameter :: TO_STRING = M

call initialize_udunits_system(rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to initialize')
@assertEqual(_SUCCESS, status, 'Failed to initialize')
call get_converter(conv, FROM_STRING, TO_STRING, rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to get converter')
@assertEqual(_SUCCESS, status, 'Failed to get converter')
actual = conv%convert(FROM)
@assertEqual(actual, EXPECTED, 'Actual does not equal expected.')
call conv%free()
Expand All @@ -65,9 +66,9 @@ contains
character(len=*), parameter :: TO_STRING = M

call initialize_udunits_system(rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to initialize')
@assertEqual(_SUCCESS, status, 'Failed to initialize')
call get_converter(conv, FROM_STRING, TO_STRING, rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to get converter')
@assertEqual(_SUCCESS, status, 'Failed to get converter')
actual = conv%convert(FROM)
@assertEqual(actual, EXPECTED, 'Actual does not equal expected.')
call conv%free()
Expand All @@ -86,9 +87,9 @@ contains
character(len=*), parameter :: TO_STRING = M

call initialize_udunits_system(rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to initialize')
@assertEqual(_SUCCESS, status, 'Failed to initialize')
call get_converter(conv, FROM_STRING, TO_STRING, rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to get converter')
@assertEqual(_SUCCESS, status, 'Failed to get converter')
actual = conv%convert(FROM)
@assertEqual(actual, EXPECTED, 'Actual does not equal expected.')
call conv%free()
Expand All @@ -107,9 +108,9 @@ contains
character(len=*), parameter :: TO_STRING = M

call initialize_udunits_system(rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to initialize')
@assertEqual(_SUCCESS, status, 'Failed to initialize')
call get_converter(conv, FROM_STRING, TO_STRING, rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to get converter')
@assertEqual(_SUCCESS, status, 'Failed to get converter')
actual = conv%convert(FROM)
@assertEqual(actual, EXPECTED, 'Actual does not equal expected.')
call conv%free()
Expand Down
11 changes: 6 additions & 5 deletions udunits2f/tests/Test_udunits2f.pf
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "MAPL_ErrLog.h"
module Test_udunits2f

use funit
Expand Down Expand Up @@ -51,7 +52,7 @@ contains
integer(ut_status) :: status

call initialize_udunits_system(rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to initialize')
@assertEqual(_SUCCESS, status, 'Failed to initialize')
unit1 = UDUnit(KM)
@assertFalse(unit1%is_free(), 'ut_unit is not set (default encoding)')

Expand All @@ -68,7 +69,7 @@ contains
integer(ut_status) :: status

call initialize_udunits_system(rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to initialize')
@assertEqual(_SUCCESS, status, 'Failed to initialize')
unit1 = UDUnit(KM)
unit2 = UDUnit(M)
conv = Converter(unit1, unit2)
Expand Down Expand Up @@ -124,7 +125,7 @@ contains
logical :: convertible

call initialize_udunits_system(rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to initialize')
@assertEqual(_SUCCESS, status, 'Failed to initialize')
unit1 = UDUnit(KM)
unit2 = UDUnit(M)
convertible = are_convertible(unit1, unit2, rc=status)
Expand All @@ -147,15 +148,15 @@ contains
logical :: convertible

call initialize_udunits_system(rc=status)
@assertEqual(UT_SUCCESS, status, 'Failed to initialize')
@assertEqual(_SUCCESS, status, 'Failed to initialize')
unit1 = UDUnit(KM)
unit2 = UDUnit(S)
convertible = are_convertible(unit1, unit2, rc=status)
@assertFalse(convertible, 'Units are not convertible.')
if(.not. convertible) then
@assertFalse(status == UT_BAD_ARG, 'One of the units is null.')
@assertFalse(status == UT_NOT_SAME_SYSTEM, 'Units belong to different systems.')
@assertTrue(status == UT_SUCCESS, 'Units are not convertible.')
@assertTrue(status == _SUCCESS, 'Units are not convertible.')
end if

call unit1%free()
Expand Down

0 comments on commit e1f18d4

Please sign in to comment.