Bug fix in m_data_input.f90

[Malmahrouqi3]

User description

Description

(#902) introduced a bug where whenever --no-mpi was utilized, mpi-specific variables/subroutines would still be present/used in m_data_input.f90. The PR attempts to fix it by re-arranging MFC_MPI if enclosures (#ifdef MFC_MPI .... #endif). With the new changes, ./mfc.sh build -t post_process --no-mpi works nicely.

---

PR Type

Bug fix

---

Description

• Fixed MPI preprocessor directive placement in data input module

• Resolved compilation issues when building with --no-mpi flag

• Reorganized #ifdef MFC_MPI blocks to properly isolate MPI-specific code

---

Changes diagram

flowchart LR
  A["MPI code mixed with non-MPI"] --> B["Reorganize #ifdef blocks"]
  B --> C["MPI code properly isolated"]
  C --> D["--no-mpi builds successfully"]

Loading

---

Changes walkthrough 📝

Relevant files

Bug fix

m_data_input.f90 Fix MPI preprocessor directive placement                                  src/post_process/m_data_input.f90 Added #ifdef MFC_MPI directive before s_setup_mpi_io_params subroutine Added corresponding #endif after the subroutine Moved MPI variable declarations inside #ifdef MFC_MPI blocks in s_read_ib_data_files Reorganized MPI preprocessor directives in s_read_parallel_conservative_data +7/-6

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Missing Guard The subroutine s_read_ib_data_files contains MPI-specific code that calls MPI functions but is not fully protected by MPI preprocessor directives. The parallel_io branch uses MPI file operations without proper guards. if (parallel_io) then write (file_loc, '(A)') trim(file_loc_base)//'ib.dat' Incomplete Protection The s_read_parallel_conservative_data subroutine is now wrapped in MFC_MPI guards but may be called from non-MPI code paths, potentially causing compilation or runtime issues when MPI is disabled. #ifdef MFC_MPI !> Helper subroutine to read parallel conservative variable data !! @param t_step Current time-step !! @param m_MOK, n_MOK, p_MOK MPI offset kinds for dimensions !! @param WP_MOK, MOK, str_MOK, NVARS_MOK Other MPI offset kinds impure subroutine s_read_parallel_conservative_data(t_step, m_MOK, n_MOK, p_MOK, WP_MOK, MOK, str_MOK, NVARS_MOK) integer, intent(in) :: t_step integer(KIND=MPI_OFFSET_KIND), intent(inout) :: m_MOK, n_MOK, p_MOK integer(KIND=MPI_OFFSET_KIND), intent(inout) :: WP_MOK, MOK, str_MOK, NVARS_MOK integer :: ifile, ierr, data_size integer, dimension(MPI_STATUS_SIZE) :: status integer(KIND=MPI_OFFSET_KIND) :: disp, var_MOK character(LEN=path_len + 2*name_len) :: file_loc logical :: file_exist character(len=10) :: t_step_string integer :: i if (file_per_process) then call s_int_to_str(t_step, t_step_string) ! Open the file to read conservative variables write (file_loc, '(I0,A1,I7.7,A)') t_step, '_', proc_rank, '.dat' file_loc = trim(case_dir)//'/restart_data/lustre_'//trim(t_step_string)//trim(mpiiofs)//trim(file_loc) inquire (FILE=trim(file_loc), EXIST=file_exist) if (file_exist) then call MPI_FILE_OPEN(MPI_COMM_SELF, file_loc, MPI_MODE_RDONLY, mpi_info_int, ifile, ierr) call s_setup_mpi_io_params(data_size, m_MOK, n_MOK, p_MOK, WP_MOK, MOK, str_MOK, NVARS_MOK) ! Read the data for each variable if (bubbles_euler .or. elasticity .or. mhd) then do i = 1, sys_size var_MOK = int(i, MPI_OFFSET_KIND) call MPI_FILE_READ_ALL(ifile, MPI_IO_DATA%var(i)%sf, data_size, & mpi_p, status, ierr) end do else do i = 1, adv_idx%end var_MOK = int(i, MPI_OFFSET_KIND) call MPI_FILE_READ_ALL(ifile, MPI_IO_DATA%var(i)%sf, data_size, & mpi_p, status, ierr) end do end if call s_mpi_barrier() call MPI_FILE_CLOSE(ifile, ierr) call s_read_ib_data_files(trim(case_dir)//'/restart_data'//trim(mpiiofs)) else call s_mpi_abort('File '//trim(file_loc)//' is missing. Exiting.') end if else ! Open the file to read conservative variables write (file_loc, '(I0,A)') t_step, '.dat' file_loc = trim(case_dir)//'/restart_data'//trim(mpiiofs)//trim(file_loc) inquire (FILE=trim(file_loc), EXIST=file_exist) if (file_exist) then call MPI_FILE_OPEN(MPI_COMM_WORLD, file_loc, MPI_MODE_RDONLY, mpi_info_int, ifile, ierr) call s_setup_mpi_io_params(data_size, m_MOK, n_MOK, p_MOK, WP_MOK, MOK, str_MOK, NVARS_MOK) ! Read the data for each variable do i = 1, sys_size var_MOK = int(i, MPI_OFFSET_KIND) ! Initial displacement to skip at beginning of file disp = m_MOK*max(MOK, n_MOK)*max(MOK, p_MOK)*WP_MOK*(var_MOK - 1) call MPI_FILE_SET_VIEW(ifile, disp, mpi_p, MPI_IO_DATA%view(i), & 'native', mpi_info_int, ierr) call MPI_FILE_READ_ALL(ifile, MPI_IO_DATA%var(i)%sf, data_size, & mpi_p, status, ierr) end do call s_mpi_barrier() call MPI_FILE_CLOSE(ifile, ierr) call s_read_ib_data_files(trim(case_dir)//'/restart_data'//trim(mpiiofs)) else call s_mpi_abort('File '//trim(file_loc)//' is missing. Exiting.') end if end if end subroutine s_read_parallel_conservative_data #endif

[qodo-code-review]

PR Code Suggestions ✨

No code suggestions found for the PR.

[Copilot]

Pull Request Overview

This PR fixes a bug where MPI-specific code in m_data_input.f90 was still compiled when using --no-mpi. It does so by wrapping the MPI helper subroutines and related declarations in #ifdef MFC_MPI guards.

• Wrapped s_setup_mpi_io_params subroutine in an #ifdef MFC_MPI block
• Added #ifdef MFC_MPI around MPI declarations (status, disp) in s_read_ib_data_files
• Enclosed entire s_read_parallel_conservative_data subroutine within #ifdef MFC_MPI

Comments suppressed due to low confidence (2)

src/post_process/m_data_input.f90:433

• [nitpick] Since this #ifdef now controls compilation of s_read_parallel_conservative_data, add or update a CI test to ensure the subroutine is excluded when building with --no-mpi to prevent accidental regressions.

#ifdef MFC_MPI

src/post_process/m_data_input.f90:111

• [nitpick] The subroutine s_setup_mpi_io_params has more than 6 parameters, exceeding the project guideline. Consider grouping related parameters into a derived type to reduce the argument count.

#ifdef MFC_MPI

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 43.71%. Comparing base (adcc0dd) to head (69eb3b1).
Report is 8 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #938   +/-   ##
=======================================
  Coverage   43.71%   43.71%           
=======================================
  Files          68       68           
  Lines       18360    18360           
  Branches     2292     2292           
=======================================
  Hits         8026     8026           
  Misses       8945     8945           
  Partials     1389     1389           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.