Skip to content

Code tidying #907

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

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Code tidying #907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
73ad673
Code Tidying
XZTian64 Jun 28, 2025
a60547b
Merge remote-tracking branch 'upstream/master' into CodeTidying
XZTian64 Jun 28, 2025
95474cf
Fixed for No double precision intrinsics warning
XZTian64 Jun 29, 2025
e3d0040
Another fix
XZTian64 Jun 29, 2025
082b2ba
Format
XZTian64 Jun 29, 2025
7a287ce
Possible fix for Cray
XZTian64 Jul 1, 2025
9282c6d
Merge remote-tracking branch 'upstream/master' into CodeTidying
XZTian64 Jul 1, 2025
249fd15
Possible fix 2
XZTian64 Jul 1, 2025
446d38f
Merge branch 'master' into CodeTidying
XZTian64 Jul 1, 2025
5531fa2
Merge branch 'master' into CodeTidying
sbryngelson Jul 3, 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
20 changes: 7 additions & 13 deletions src/common/m_boundary_common.fpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1028,9 +1028,9 @@
integer, intent(in) :: bc_dir, bc_loc
integer, intent(in) :: k, l

#ifdef MFC_SIMULATION

Check warning on line 1031 in src/common/m_boundary_common.fpp

View check run for this annotation

Codecov / codecov/patch

src/common/m_boundary_common.fpp#L1031 

Added line #L1031 was not covered by tests
integer :: j, i

#ifdef MFC_SIMULATION
if (bc_dir == 1) then !< x-direction
if (bc_loc == -1) then !bc_x%beg
do i = 1, sys_size
Expand Down Expand Up @@ -1520,7 +1520,7 @@

character(LEN=*), intent(in) :: step_dirpath

integer :: dir, loc, i
integer :: dir, loc
character(len=path_len) :: file_path

character(len=10) :: status
Expand Down Expand Up @@ -1561,12 +1561,10 @@
integer :: dir, loc
character(len=path_len) :: file_loc, file_path

character(len=10) :: status

#ifdef MFC_MPI
integer :: ierr
integer :: file_id
integer :: offset
integer(KIND=MPI_ADDRESS_KIND) :: offset
character(len=7) :: proc_rank_str
logical :: dir_check

Expand Down Expand Up @@ -1625,8 +1623,6 @@
logical :: file_exist
character(len=path_len) :: file_path

character(len=10) :: status

! Read bc_types
file_path = trim(step_dirpath)//'/bc_type.dat'
inquire (FILE=trim(file_path), EXIST=file_exist)
Expand Down Expand Up @@ -1668,12 +1664,10 @@
integer :: dir, loc
character(len=path_len) :: file_loc, file_path

character(len=10) :: status

#ifdef MFC_MPI
integer :: ierr
integer :: file_id
integer :: offset
integer(KIND=MPI_ADDRESS_KIND) :: offset
character(len=7) :: proc_rank_str
logical :: dir_check

Expand All @@ -1696,7 +1690,7 @@
file_path = trim(file_loc)//'/bc_'//trim(proc_rank_str)//'.dat'
call MPI_File_open(MPI_COMM_SELF, trim(file_path), MPI_MODE_RDONLY, MPI_INFO_NULL, file_id, ierr)

offset = 0
offset = int(0, KIND=MPI_ADDRESS_KIND)

! Read bc_types
do dir = 1, num_dims
Expand Down Expand Up @@ -1788,9 +1782,9 @@
!! boundary locations and cell-width distributions, based on
!! the boundary conditions.
subroutine s_populate_grid_variables_buffers

#ifndef MFC_PRE_PROCESS
integer :: i !< Generic loop iterator

#endif
#ifdef MFC_SIMULATION
! Required for compatibility between codes
type(int_bounds_info) :: offset_x, offset_y, offset_z
Expand Down
5 changes: 3 additions & 2 deletions src/common/m_checker_common.fpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,9 @@ contains
!> Checks constraints on the surface tension parameters.
!! Called by s_check_inputs_common for all three stages
impure subroutine s_check_inputs_surface_tension

#ifdef MFC_PRE_PROCESS
integer :: i
#endif

@:PROHIBIT(surface_tension .and. sigma < 0._wp, &
"sigma must be greater than or equal to zero")
Expand All @@ -339,7 +340,7 @@ contains
@:PROHIBIT(surface_tension .and. f_is_default(patch_icpp(i)%cf_val), &
"patch_icpp(i)%cf_val must be set if surface_tension is enabled")
end do
#endif MFC_PRE_PROCESS
#endif

end subroutine s_check_inputs_surface_tension

Expand Down
15 changes: 11 additions & 4 deletions src/common/m_finite_differences.fpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,20 @@
!! @param s_cc Locations of the cell-centers in the s-coordinate direction
!! @param fd_coeff_s Finite-diff. coefficients in the s-coordinate direction
pure subroutine s_compute_finite_difference_coefficients(q, s_cc, fd_coeff_s, buff_size, &
fd_number_in, fd_order_in, offset_s)
fd_order_in, fd_number_in, offset_s)

integer :: lB, lE !< loop bounds
integer, intent(IN) :: q
integer, intent(IN) :: buff_size, fd_number_in, fd_order_in
integer, intent(IN) :: buff_size, fd_order_in
integer, optional, intent(IN) :: fd_number_in

type(int_bounds_info), optional, intent(IN) :: offset_s
real(wp), allocatable, dimension(:, :), intent(INOUT) :: fd_coeff_s

real(wp), &
dimension(-buff_size:q + buff_size), &
intent(IN) :: s_cc

integer :: fd_number
integer :: i !< Generic loop iterator

if (present(offset_s)) then
Expand All @@ -88,10 +90,15 @@
lB = 0
lE = q
end if
if (present(fd_number_in)) then
fd_number = fd_number_in
else
fd_number = 2

Check warning on line 96 in src/common/m_finite_differences.fpp

View check run for this annotation

Codecov / codecov/patch

src/common/m_finite_differences.fpp#L96 

Added line #L96 was not covered by tests
end if

#ifdef MFC_POST_PROCESS
if (allocated(fd_coeff_s)) deallocate (fd_coeff_s)
allocate (fd_coeff_s(-fd_number_in:fd_number_in, lb:lE))
allocate (fd_coeff_s(-fd_number:fd_number, lb:lE))
#endif

! Computing the 1st order finite-difference coefficients
Expand Down
16 changes: 12 additions & 4 deletions src/common/m_helper_basic.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
!> This procedure checks if two floating point numbers of wp are within tolerance.
!! @param a First number.
!! @param b Second number.
!! @param tol_input Relative error (default = 1.e-10_wp).
!! @param tol_input Relative error (default = 1.e-10_wp for double and 1e-6 for single).
!! @return Result of the comparison.
logical pure elemental function f_approx_equal(a, b, tol_input) result(res)
!$acc routine seq
Expand All @@ -32,7 +32,11 @@
if (present(tol_input)) then
tol = tol_input
else
tol = 1.e-10_wp
if (wp == selected_real_kind(15, 307)) then
Copy link
Member

Choose a reason for hiding this comment

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

does this even work? have you tested it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I forgot to check the comments. So this one aims to choose tolerance on different precisions, as single precision can only reach to 1e-8. If the tol is not choosen properly, like choose 1e-6 for double precision, will make the test fail. For now this passes both single and double tests.

Copy link
Member

Choose a reason for hiding this comment

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

yeah i understand its use case, it's a good fix. but please change to the variables single_precision and double_precision per my other comment. they're already defined.

tol = 1.e-10_wp
else if (wp == selected_real_kind(6, 37)) then
tol = 1.e-6_wp
end if
end if

if (a == b) then
Expand All @@ -47,7 +51,7 @@
!> This procedure checks if the point numbers of wp belongs to another array are within tolerance.
!! @param a First number.
!! @param b Array that contains several point numbers.
!! @param tol_input Relative error (default = 1e-10_wp).
!! @param tol_input Relative error (default = 1.e-10_wp for double and 1e-6 for single).
!! @return Result of the comparison.
logical pure function f_approx_in_array(a, b, tol_input) result(res)
!$acc routine seq
Expand All @@ -62,7 +66,11 @@
if (present(tol_input)) then
tol = tol_input
else
tol = 1e-10_wp
if (wp == selected_real_kind(15, 307)) then
Copy link
Member

Choose a reason for hiding this comment

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

same question here. does this if statement work as expected? wp can be either double or single but does it do what you expect?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If the wp does not match any one of the if statement, not tol will be assigned. The program will abort. Also the wp is defined here:

    integer, parameter :: single_precision = selected_real_kind(6, 37)
    integer, parameter :: double_precision = selected_real_kind(15, 307)
#ifdef MFC_SINGLE_PRECISION
    integer, parameter :: wp = single_precision  ! Change to single_precision if needed
#else
    integer, parameter :: wp = double_precision
#endif

As the tests pass smoothly, I believe this work as expected.

Copy link
Member

Choose a reason for hiding this comment

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

change this from wp == selected_real_kind() to wp == single_precision elseif wp == double_precision...

tol = 1.e-10_wp

Check warning on line 70 in src/common/m_helper_basic.f90

View check run for this annotation

Codecov / codecov/patch

src/common/m_helper_basic.f90#L69-L70 

Added lines #L69 - L70 were not covered by tests
else if (wp == selected_real_kind(6, 37)) then
tol = 1.e-6_wp
end if
end if

do i = 1, size(b)
Expand Down
14 changes: 12 additions & 2 deletions src/common/m_mpi_common.fpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,17 @@ contains
type(scalar_field), intent(in), optional :: beta

integer, dimension(num_dims) :: sizes_glb, sizes_loc
integer, dimension(1) :: airfoil_glb, airfoil_loc, airfoil_start
#ifndef MFC_POST_PROCESS

integer, dimension(1) :: airfoil_glb, airfoil_loc, airfoil_start
#endif
#ifdef MFC_MPI

! Generic loop iterator
integer :: i, j
integer :: i
#ifndef MFC_POST_PROCESS
integer :: j
#endif

!Altered system size for the lagrangian subgrid bubble model
integer :: alt_sys
Expand Down Expand Up @@ -363,6 +368,11 @@ contains
real(wp), intent(out) :: vcfl_max_glb
real(wp), intent(out) :: Rc_min_glb

! Initiate the global variables to the local values to avoid warnings
Copy link
Member

Choose a reason for hiding this comment

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

change comment to 'Initiate global variables'

icfl_max_glb = icfl_max_loc
vcfl_max_glb = vcfl_max_loc
Rc_min_glb = Rc_min_loc

#ifdef MFC_SIMULATION
#ifdef MFC_MPI

Expand Down
19 changes: 12 additions & 7 deletions src/common/m_nvtx.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,24 @@

subroutine nvtxStartRange(name, id)
character(kind=c_char, len=*), intent(IN) :: name
integer, intent(IN), optional :: id
integer, intent(in), optional :: id
integer :: id_color
#if defined(MFC_OpenACC) && defined(__PGI)
type(nvtxEventAttributes) :: event
#endif
if (present(id)) then
id_color = col(mod(id, 7) + 1)

Check warning on line 64 in src/common/m_nvtx.f90

View check run for this annotation

Codecov / codecov/patch

src/common/m_nvtx.f90#L64 

Added line #L64 was not covered by tests
end if
tempName = trim(name)//c_null_char

Check warning on line 66 in src/common/m_nvtx.f90

View check run for this annotation

Codecov / codecov/patch

src/common/m_nvtx.f90#L66 

Added line #L66 was not covered by tests

#if defined(MFC_OpenACC) && defined(__PGI)

tempName = trim(name)//c_null_char

if (.not. present(id)) then
call nvtxRangePush(tempName)
else
event%color = col(mod(id, 7) + 1)
if (present(id)) then
Copy link
Member

Choose a reason for hiding this comment

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

does this work, too? you need to run nsys with cpu and gpu modes to find out

event%color = id_color
event%message = c_loc(tempName)
call nvtxRangePushEx(event)
else
call nvtxRangePush(tempName)
end if

#endif
Expand Down
Loading
Loading