Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix real32 gnu issue #340

Merged
merged 9 commits into from
Jan 10, 2025
Merged

fix real32 gnu issue #340

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
dc1beee
fix real32 gnu issue
Dec 16, 2024
87fb93b
fix samples bug
peverwhee Dec 17, 2024
5765ba8
add errmsg to check_allocate; cleanup
peverwhee Dec 19, 2024
0bcfc85
Merge remote-tracking branch 'ESCOMP/development' into history-bugfix…
peverwhee Dec 19, 2024
0748e03
cam_grid_write_dist_array expects array of dimension sizes
peverwhee Dec 19, 2024
fe50039
remove intel snapshot tests
peverwhee Dec 19, 2024
a4dec85
remove min_difference for kessler
peverwhee Dec 19, 2024
b1173bd
fix bad mod logic
peverwhee Jan 8, 2025
e9de7c7
merge to head of development
peverwhee Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 40 additions & 14 deletions src/history/cam_hist_file.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1415,7 +1415,7 @@ subroutine config_write_time_dependent_variables(this, restart)
! Increment samples
this%num_samples = this%num_samples + 1

start = mod(this%num_samples, this%max_frames) + 1
start = mod(this%num_samples, this%max_frames)
count1 = 1

is_initfile = (this%hfile_type == hfile_type_init_value)
Expand Down Expand Up @@ -1485,7 +1485,8 @@ subroutine config_write_time_dependent_variables(this, restart)
split_file_index == instantaneous_file_index .and. .not. restart) then
cycle
end if
call this%write_field(this%field_list(field_idx), split_file_index, restart, start, field_idx)
call this%write_field(this%field_list(field_idx), split_file_index, restart, start, field_idx, &
this%precision())
end do
end do
call t_stopf ('write_field')
Expand All @@ -1495,20 +1496,22 @@ end subroutine config_write_time_dependent_variables
! ========================================================================

subroutine config_write_field(this, field, split_file_index, restart, &
sample_index, field_index)
sample_index, field_index, field_precision)
use pio, only: PIO_OFFSET_KIND, pio_setframe
use hist_buffer, only: hist_buffer_t
use hist_api, only: hist_buffer_norm_value
use cam_grid_support, only: cam_grid_write_dist_array
use cam_abortutils, only: check_allocate, endrun
use hist_field, only: hist_field_info_t
use shr_kind_mod, only: r4 => shr_kind_r4
! Dummy arguments
class(hist_file_t), intent(inout) :: this
type(hist_field_info_t), intent(inout) :: field
integer, intent(in) :: split_file_index
logical, intent(in) :: restart
integer, intent(in) :: sample_index
integer, intent(in) :: field_index
character(len=*), intent(in) :: field_precision

! Local variables
integer, allocatable :: field_shape(:) ! Field file dim sizes
Expand All @@ -1522,7 +1525,8 @@ subroutine config_write_field(this, field, split_file_index, restart, &
type(var_desc_t) :: varid
integer :: field_decomp
integer :: idx
real(r8), allocatable :: field_data(:,:)
real(r8), allocatable :: field_data_r8(:,:)
real(r4), allocatable :: field_data_r4(:,:)
class(hist_buffer_t), pointer :: buff_ptr
character(len=*), parameter :: subname = 'config_write_field: '

Expand All @@ -1533,11 +1537,21 @@ subroutine config_write_field(this, field, split_file_index, restart, &
end_dims = field%end_dims()
frank = size(field_shape)
if (frank == 1) then
allocate(field_data(end_dims(1) - beg_dims(1) + 1, 1), stat=ierr)
call check_allocate(ierr, subname, 'field_data', file=__FILE__, line=__LINE__-1)
if (trim(field_precision) == 'REAL32') then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't we recently learn that you don't need to trim the string variable in order to do the string comparison? If so then I would vote to remove the trim calls here and below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's only true for comparison vs an array of strings (e.g. "any" syntax) where you don't have to trim each array element. Like:

if (any(freq_types_to_check == trim(this%output_freq_type))) then

but i may be misremembering!

allocate(field_data_r4(end_dims(1) - beg_dims(1) + 1, 1), stat=ierr)
call check_allocate(ierr, subname, 'field_data_r4', file=__FILE__, line=__LINE__-1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that check_allocate supports passing in errmsg as well can we do that here and below? Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added!

else
allocate(field_data_r8(end_dims(1) - beg_dims(1) + 1, 1), stat=ierr)
call check_allocate(ierr, subname, 'field_data_r8', file=__FILE__, line=__LINE__-1)
end if
else
allocate(field_data(end_dims(1) - beg_dims(1) + 1, field_shape(2)), stat=ierr)
call check_allocate(ierr, subname, 'field_data', file=__FILE__, line=__LINE__-1)
if (trim(field_precision) == 'REAL32') then
allocate(field_data_r4(end_dims(1) - beg_dims(1) + 1, field_shape(2)), stat=ierr)
call check_allocate(ierr, subname, 'field_data_r4', file=__FILE__, line=__LINE__-1)
else
allocate(field_data_r8(end_dims(1) - beg_dims(1) + 1, field_shape(2)), stat=ierr)
call check_allocate(ierr, subname, 'field_data_r8', file=__FILE__, line=__LINE__-1)
end if
end if
! Shape of array
dimind = field%dimensions()
Expand All @@ -1556,13 +1570,25 @@ subroutine config_write_field(this, field, split_file_index, restart, &
call pio_setframe(this%hist_files(split_file_index), varid, int(sample_index,kind=PIO_OFFSET_KIND))
buff_ptr => field%buffers
if (frank == 1) then
call hist_buffer_norm_value(buff_ptr, field_data(:,1))
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1: frank), &
field_shape, field_data(:,1), varid)
if (trim(field_precision) == 'REAL32') then
call hist_buffer_norm_value(buff_ptr, field_data_r4(:,1))
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1: frank), &
field_shape, field_data_r4(:,1), varid)
else
call hist_buffer_norm_value(buff_ptr, field_data_r8(:,1))
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1: frank), &
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we know frank equals one then I think we can simplify this slightly to the following:

Suggested change
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1: frank), &
field_shape, field_data_r4(:,1), varid)
else
call hist_buffer_norm_value(buff_ptr, field_data_r8(:,1))
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1: frank), &
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1), &
field_shape, field_data_r4(:,1), varid)
else
call hist_buffer_norm_value(buff_ptr, field_data_r8(:,1))
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1), &

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cam_grid_write_dist_array is expecting an array, so updated to

field_decomp, (/dim_sizes(1)/),

field_shape, field_data_r8(:,1), varid)
end if
else
call hist_buffer_norm_value(buff_ptr, field_data)
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1: frank), &
field_shape, field_data, varid)
if (trim(field_precision) == 'REAL32') then
call hist_buffer_norm_value(buff_ptr, field_data_r4)
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1: frank), &
field_shape, field_data_r4, varid)
else
call hist_buffer_norm_value(buff_ptr, field_data_r8)
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1: frank), &
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very silly nit-pick but can we remove the space between 1: and frank here?

Suggested change
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1: frank), &
field_shape, field_data_r4, varid)
else
call hist_buffer_norm_value(buff_ptr, field_data_r8)
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1: frank), &
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1:frank), &
field_shape, field_data_r4, varid)
else
call hist_buffer_norm_value(buff_ptr, field_data_r8)
call cam_grid_write_dist_array(this%hist_files(split_file_index), field_decomp, dim_sizes(1:frank), &

Copy link
Collaborator Author

@peverwhee peverwhee Dec 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the picked nit has been squashed (?)

...updated

field_shape, field_data_r8, varid)
end if
end if
end do

Expand Down
2 changes: 1 addition & 1 deletion test/existing-test-failures.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ SMS_Ln2.mpasa480_mpasa480.FKESSLER.derecho_gnu.cam-outfrq_kessler_mpas_derecho (
SMS_Ln9.ne5pg3_ne5pg3_mg37.FCAM7.derecho_intel.cam-outfrq_se_cslam_analy_ic (Overall: PEND) details:
SMS_Ln9.ne5pg3_ne5pg3_mg37.FCAM7.derecho_gnu.cam-outfrq_se_cslam_analy_ic (Overall: PEND) details:
- build failure due to dadadj_apply_qv_tendency removal, needs to be updated to use constituent tendency updater atmospheric_physics#179
- also expected runtime failure CAM-SIMA#335
- also expected runtime failure until we have a CCPP-Framework tag from https://github.com/NCAR/ccpp-framework/pull/618
Loading