compdyn / partmc

Particle-resolved stochastic atmospheric aerosol model
http://lagrange.mechse.illinois.edu/partmc/
GNU General Public License v2.0
28 stars 16 forks source link

Add dockerfile mpi #163

Closed jcurtis2 closed 2 years ago

jcurtis2 commented 3 years ago

Adding a dockerfile for testing of PartMC compiled with MPI.

jcurtis2 commented 3 years ago

@mwest1066 if you could take a look at this warning in the Dockerfile.mpi build that is turning into an error in the build process with our compiler flags, I don't quite see why gfortran complains about it (after working through all the logic of that subroutine). It complains about the following lines:

https://github.com/compdyn/partmc/blob/0e8cf41bf2d783005f34998789862ce545d5f6c1/src/bin_grid.F90#L370-L371

saying "Error: Impure function 'pmc_mpi_allequal_real' at (1) might not be evaluated [-Werror=function-elimination".

There's a few ways to avoid this problem but I wasn't sure if I was missing something (we can split the if statement into two) or we can get the logicals separately and compare:

    logical :: same_bin_min, same_bin_max
    same_bin_min = pmc_mpi_allequal_real(val%edges(1))
    same_bin_max = pmc_mpi_allequal_real(val%edges(bin_grid_size(val)))
    if (bin_min .and. bin_max) then
       pmc_mpi_allequal_bin_grid = .true.
    else
       pmc_mpi_allequal_bin_grid = .false.
    end if
mwest1066 commented 3 years ago

I think your solution with same_bin_min, same_bin_max is good. Let's use that.

The reason that it's warning is:

  1. If pmc_mpi_allequal_real(val%edges(1)) is false
  2. Then Fortran will short-circuit the evaluation of the .and.
  3. Thus it will not even compute pmc_mpi_allequal_real(val%edges(bin_grid_size(val)))
  4. But pmc_mpi_allequal_real() is "impure" (that is, it is not marked as "pure")
  5. Therefore the compiler can't be sure that the possibly-missing second call to pmc_mpi_allequal_real() wasn't having an important side-effect
  6. Thus we get a warning/error

Your fix with same_bin_min, same_bin_max solves this problem because it forces the code to always call pmc_mpi_allequal_real() twice, avoiding the short-circuit .and.. The nested-if solution can short-circuit (if the outside if is false then it won't try the inner if) but this is ok because it's explicitly in the code, so the compiler won't warn.

jcurtis2 commented 2 years ago

https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources

Github hardware is a currently a 2 core CPU so the MPI code was adjusted to 2 cores for now so tests pass.