time manager mixed precision #1091
Conversation
| module procedure get_cal_time_r8 | ||
| end interface | ||
|
|
||
| public :: get_cal_time |
There was a problem hiding this comment.
Why make this public underneath interface?
There was a problem hiding this comment.
I don't think i moved that on purpose, but the order doesn't matter for public statements.
| @@ -150,28 +109,28 @@ contains | |||
| !! | |||
| !! @note This option was originally coded to allow noleap calendar as input when | |||
| !! the julian calendar was in effect by the time_manager. | |||
| function get_cal_time(time_increment, units, calendar, permit_calendar_conversion) | |||
| real, intent(in) :: time_increment | |||
| function GET_CAL_TIME(time_increment, units, calendar, permit_calendar_conversion) result(cal_time) | |||
| function get_cal_time(time_increment, units, calendar, permit_calendar_conversion) | ||
| real, intent(in) :: time_increment | ||
| function GET_CAL_TIME(time_increment, units, calendar, permit_calendar_conversion) result(cal_time) | ||
| real(FMS_TM_KIND), intent(in) :: time_increment |
There was a problem hiding this comment.
FMS_TM_KIND_ for all the FMS_TM_KIND here
| character(len=*), intent(in) :: units | ||
| character(len=*), intent(in) :: calendar | ||
| logical, intent(in), optional :: permit_calendar_conversion | ||
| type(time_type) :: get_cal_time | ||
| type(time_type) :: cal_time |
There was a problem hiding this comment.
I changed the result to cal_time so it wouldn't have to be replaced by the macro each time it's used.
| @@ -313,50 +272,33 @@ else if(lowercase(units(1:12)) == 'months since') then | |||
| increment_days = floor(dt/86400) | |||
| increment_seconds = int(dt - increment_days*86400) | |||
| else | |||
There was a problem hiding this comment.
between lines 244-273, there are several integers that need to be real(#,FMS_TM_KIND_) IT SEEMS
There was a problem hiding this comment.
Yeah i skipped these cause I don't think the precision makes a difference here, especially for the floor ones. I'll add those in to be consistent though, might as well be explicit with it.
| @@ -313,50 +272,33 @@ else if(lowercase(units(1:12)) == 'months since') then | |||
| increment_days = floor(dt/86400) | |||
| increment_seconds = int(dt - increment_days*86400) | |||
There was a problem hiding this comment.
real(increment_days,FMS_TM_KIND_)
time_manager/time_manager.F90
Outdated
| ticks_new = modulo(ticks,ticks_per_second) | ||
| days_new = days + floor(seconds_new/real(seconds_per_day)) | ||
| days_new = days + floor(seconds_new/real(seconds_per_day, r8_kind)) |
| @@ -456,7 +488,7 @@ function get_tick_from_string(string, err_msg, allow_rounding, tick) | |||
| magnitude = 10**nspf | |||
| tpsfrac = ticks_per_second*fraction | |||
| if(allow_rounding) then | |||
| tick = nint((real(tpsfrac)/magnitude)) | |||
| tick = nint((real(tpsfrac, r8_kind)/magnitude)) | |||
| else | |||
| if(modulo(tpsfrac,magnitude) == 0) then | |||
| tick = tpsfrac/magnitude | |||
| @@ -456,7 +488,7 @@ function get_tick_from_string(string, err_msg, allow_rounding, tick) | |||
| magnitude = 10**nspf | |||
| tpsfrac = ticks_per_second*fraction | |||
| if(allow_rounding) then | |||
| tick = nint((real(tpsfrac)/magnitude)) | |||
| tick = nint((real(tpsfrac, r8_kind)/magnitude)) | |||
| type(time_type) :: time_scalar_mult | ||
| type(time_type), intent(in) :: time | ||
| integer, intent(in) :: n | ||
| integer :: days, seconds, ticks, num_sec | ||
| double precision :: sec_prod, tick_prod | ||
| real(r8_kind) :: sec_prod, tick_prod | ||
|
|
||
| if(.not.module_is_initialized) call time_manager_init | ||
|
|
There was a problem hiding this comment.
I remember you bringing up the double issue. Since the precision should be the same/consistent between Intel and GCC compiled code, maybe should go with real() instead? Not sure.
| write(outunit,'(a)') 'test successful: '//trim(err_msg) | ||
| endif | ||
| !! check equality between r4 and r8 returned types | ||
| if ( real_to_time_type(real(86401.1, r4_kind)) .ne. real_to_time_type(real(86401.1, r8_kind))) then |
There was a problem hiding this comment.
do you think it would make a difference if using 86401.1_r4_kind and 86401.1_r8_kind?
|
opening new pr |
Description
adds mixed precision real support to time manager.
From testing it looks like most of the assignment and operator overloads can't be overridden since there's no way to make distinct interfaces. This means time type to real conversions and divisions will be r8 and will need to be casted.
I changed the
double precisiontypes toreal(r8_kind)since its more portable for any routines that use casted integers.Fixes # (issue)
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note
any relevant details for your test configuration (e.g. compiler, OS). Include
enough information so someone can reproduce your tests.
Checklist:
make distcheckpasses