search

NCAR / DART

Data Assimilation Research Testbed
https://dart.ucar.edu/
Apache License 2.0
196 stars 145 forks source link

all_reduce_min_max subroutine should be removed from the mpi_utilities_mod files #762

Closed mjs2369 closed 1 week ago

mjs2369 commented 3 weeks ago

The is an unneeded subroutine in the mpi_utilities_mod files (mpi_utilities_mod.f90, mpif08_utilities_mod.f90, null_mpi_utilities_mod.f90)

https://github.com/NCAR/DART/blob/8c549735550e7120ee926f3b194ee8f0cba5ada7/assimilation_code/modules/utilities/mpif08_utilities_mod.f90#L1470-L1481

all_reduce_min_max just calls broadcast_minmax

It looks like the only place that all_reduce_min_max is still called is in the wrf model_mod: https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/models/wrf/model_mod.f90#L5953

Finally, it would be better to change the name for broadcast_minmax back to all_reduce_min_max as this is more descriptive of what the subroutine is actually doing; there is no broadcast in this routine.

Error Message

Please provide any error messages. N/A

Which model(s) are you working with?

All models

Screenshots

If applicable, add screenshots to help explain your problem.

Version of DART

Which version of DART are you using? You can find the version using git describe --tags
v11.8.2

Have you modified the DART code?

No

Build information

Please describe:

  1. The machine you are running on (e.g. windows laptop, NSF NCAR supercomputer Derecho).
  2. The compiler you are using (e.g. gnu, intel).
    All machines and compilers
nancycollins commented 3 weeks ago

i know there isn't a broadcast in the "broadcast_minmax", but the result is that the global min/max appears on all tasks after the call. many people don't understand what 'all_reduce' indicates - so if you have a better name for it, that's what i'd vote for. the 'broadcast_minmax' name was intended to be more user-helpful. the mpi details aren't easy for a lot of folks. but do whatever makes the most sense to you.