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.

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

Particle module remastered. #309

Merged
merged 33 commits into from
Nov 12, 2024
Merged

Particle module remastered. #309

merged 33 commits into from
Nov 12, 2024

Conversation

fangjian19
Copy link
Contributor

The new particle module now supports MPI-IO.
Currently only inertial particles are included.
Two examples are provided, TGV with particles, and Channel flow with consntantly injection of particle, i.e., the numerical hydrogen bubble experiment. Both are inclcude in the full ctest.

Copy link
Contributor

@mathrack mathrack left a comment

Choose a reason for hiding this comment

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

I have not looked deeply at the modification in mptool.f90 and at the code particle.f90. The PR is quite large and very difficult to review. It is probably broken when the code is in single precision.

In my opinion, some components of the PR should be moved to 2decomp.

src/Case-TGV.f90 Outdated Show resolved Hide resolved
src/parameters.f90 Outdated Show resolved Hide resolved
src/tools.f90 Outdated Show resolved Hide resolved
src/tools.f90 Outdated Show resolved Hide resolved
src/tools.f90 Outdated Show resolved Hide resolved
src/xcompact3d.f90 Outdated Show resolved Hide resolved
src/particle.f90 Outdated Show resolved Hide resolved
src/particle.f90 Outdated Show resolved Hide resolved
src/particle.f90 Outdated Show resolved Hide resolved
src/particle.f90 Outdated Show resolved Hide resolved
Copy link
Contributor

@mathrack mathrack left a comment

Choose a reason for hiding this comment

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

The comments in the previous review needs to be addressed :

  1. Make sure the particles can run in single precision
  2. Use the halo subroutines available in 2decomp. If it is not possible, please explain why
  3. Do not comment the pressure rescaling in the file visu.f90
  4. Fix typos (particel)
  5. Remove commented debug code
  6. Assign default values for all the variables in the namelist ParTrack
  7. In case of error, call decomp_2d_abort
  8. Optional : check error codes returned by MPI subroutines

@fangjian19
Copy link
Contributor Author

All problems raised by @mathrack have been sorted.
module tested with single precision, including io and restart.
new boundary condition (outflow) added to particle module.

@mathrack mathrack removed their request for review October 29, 2024 13:02
Copy link
Contributor

@mathrack mathrack left a comment

Choose a reason for hiding this comment

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

I have suggested some modifications. I have not yet looked at particle.f90. I have an error with the following build command :

rm -rf ./build/ && FC=mpif90 cmake -S ./ -B ./build -DBUILD_TESTING_FULL=on -DCMAKE_BUILD_TYPE=debug

The output is :

...
[100%] Completed 'downloadBuild2decomp'
[100%] Built target downloadBuild2decomp
-- Found MPI_Fortran: /usr/bin/mpif90 (found version "3.1") 
-- Found MPI: TRUE (found version "3.1")  
-- X3D MPI_Fortran_COMPILER found: /usr/bin/mpif90
-- X3D MPI_VERSION found: 
-- X3D MPI FOUND: TRUE
-- X3D MPI INCL ALSO FOUND: 
-- X3D Path to mpirun /usr/bin/mpirun
-- COMP ID GNU
-- Fortran compiler name GNU
-- Fortran compiler version 13.2.0
-- Setting gfortran flags
-- Set New Fortran basic flags
-- Using mpi (default) IO backend
-- CMAKE_SOURCE_DIR: xxx/Incompact3d
-- PROJECT_BINARY_DIR: xxx/Incompact3d/build
CMake Error at examples/CMakeLists.txt:26 (add_subdirectory):
  The binary directory

    xxx/Incompact3d/build/examples/Particle-Tracking

  is already used to build a source directory.  It cannot be used to build
  source directory

    xxx/Incompact3d/examples/Particle-Tracking

  Specify a unique binary directory name.


-- Found Python3: /usr/bin/python3 (found version "3.12.3") found components: Interpreter 
-- Add Test TGV-Taylor-Green-vortex
-- Configuring incomplete, errors occurred!

examples/CMakeLists.txt Show resolved Hide resolved
src/Case-TGV.f90 Show resolved Hide resolved
src/parameters.f90 Outdated Show resolved Hide resolved
src/parameters.f90 Outdated Show resolved Hide resolved
src/tools.f90 Outdated Show resolved Hide resolved
src/mptool.f90 Outdated Show resolved Hide resolved
src/mptool.f90 Outdated Show resolved Hide resolved
src/mptool.f90 Outdated Show resolved Hide resolved
src/mptool.f90 Show resolved Hide resolved
src/mptool.f90 Show resolved Hide resolved
@mathrack
Copy link
Contributor

There are plenty of unresolved comments, I think there is no need for a new review right now

@mathrack mathrack removed their request for review October 30, 2024 10:00
@mathrack mathrack removed their request for review October 30, 2024 10:11
@fangjian19
Copy link
Contributor Author

I have suggested some modifications. I have not yet looked at particle.f90. I have an error with the following build command :

rm -rf ./build/ && FC=mpif90 cmake -S ./ -B ./build -DBUILD_TESTING_FULL=on -DCMAKE_BUILD_TYPE=debug

The output is :

...
[100%] Completed 'downloadBuild2decomp'
[100%] Built target downloadBuild2decomp
-- Found MPI_Fortran: /usr/bin/mpif90 (found version "3.1") 
-- Found MPI: TRUE (found version "3.1")  
-- X3D MPI_Fortran_COMPILER found: /usr/bin/mpif90
-- X3D MPI_VERSION found: 
-- X3D MPI FOUND: TRUE
-- X3D MPI INCL ALSO FOUND: 
-- X3D Path to mpirun /usr/bin/mpirun
-- COMP ID GNU
-- Fortran compiler name GNU
-- Fortran compiler version 13.2.0
-- Setting gfortran flags
-- Set New Fortran basic flags
-- Using mpi (default) IO backend
-- CMAKE_SOURCE_DIR: xxx/Incompact3d
-- PROJECT_BINARY_DIR: xxx/Incompact3d/build
CMake Error at examples/CMakeLists.txt:26 (add_subdirectory):
  The binary directory

    xxx/Incompact3d/build/examples/Particle-Tracking

  is already used to build a source directory.  It cannot be used to build
  source directory

    xxx/Incompact3d/examples/Particle-Tracking

  Specify a unique binary directory name.


-- Found Python3: /usr/bin/python3 (found version "3.12.3") found components: Interpreter 
-- Add Test TGV-Taylor-Green-vortex
-- Configuring incomplete, errors occurred!

Problem solved by moving Particle-Tracking from test_full

@mathrack mathrack removed their request for review October 30, 2024 11:17
Copy link
Contributor

@mathrack mathrack left a comment

Choose a reason for hiding this comment

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

This is my final review, the other developers will look at the PR and at the file particle.f90. I have not looked at particle.f90.

src/mptool.f90 Outdated Show resolved Hide resolved
src/mptool.f90 Outdated Show resolved Hide resolved
src/mptool.f90 Show resolved Hide resolved
src/mptool.f90 Outdated Show resolved Hide resolved
src/mptool.f90 Outdated Show resolved Hide resolved
src/mptool.f90 Outdated Show resolved Hide resolved
@fangjian19
Copy link
Contributor Author

The merge will be the basic framework of the particle tracking module. The particles are only massless fluid's tracker.
But the module already contains some functions for heavy particles (currently not been used), such as, fluid's force, time interpolation. Further development can be carried on within the framework.
@pbartholomew08 @rfj82982 please review the particle.f90 file? @mathrack and I have gone through all other files.

@mathrack mathrack dismissed their stale review October 31, 2024 10:18

Mostly fixed

CMakeLists.txt Show resolved Hide resolved
src/mptool.f90 Outdated Show resolved Hide resolved
src/mptool.f90 Outdated Show resolved Hide resolved
src/mptool.f90 Show resolved Hide resolved
src/mptool.f90 Outdated Show resolved Hide resolved
src/particle.f90 Outdated Show resolved Hide resolved
src/particle.f90 Outdated Show resolved Hide resolved
src/particle.f90 Outdated Show resolved Hide resolved
src/particle.f90 Outdated Show resolved Hide resolved
src/particle.f90 Outdated Show resolved Hide resolved
Copy link
Collaborator

@rfj82982 rfj82982 left a comment

Choose a reason for hiding this comment

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

LGTM. I tested with GNU/Intel/NVHPC and with GNU i have also used ADIOS2 and FFTW. Everything is working with the exception of NVHPC. There is a problem with isnan function that is not part of the standard. I think a dedicated PR will be a better way to sort the issue.

@fangjian19
Copy link
Contributor Author

LGTM. I tested with GNU/Intel/NVHPC and with GNU i have also used ADIOS2 and FFTW. Everything is working with the exception of NVHPC. There is a problem with isnan function that is not part of the standard. I think a dedicated PR will be a better way to sort the issue.

Many thanks Stefano.

Copy link
Collaborator

@rfj82982 rfj82982 left a comment

Choose a reason for hiding this comment

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

LGTM

@rfj82982 rfj82982 merged commit 14ebbad into xcompact3d:master Nov 12, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants