use hash of comm_tag if not numeric

[matthiasdiener]

Could resolve illinois-ceesd/mirgecom#617

cc: @MTCam

[matthiasdiener]

I think this is probably incorrect in case hash(comm_tag) > MPI_TAG_UB (but I'm not sure if that can happen).

[inducer]

Fair, stick a modulo in there. It's a stopgap.

[inducer]

You were right: OverflowError: value too large to convert to int

[inducer]

Thx!

[MTCam]

Don't all ranks need to agree on the message-specific tags? I don't know how this example CI is passing. When I run MIRGE-Com examples by hand, and by MIRGE-Com CI, they appear to hang.

After mpiexec -n 2 python -m mpi4py pulse-mpi.py:

----- cfl=array(0.00100953)
P(101324.99999999999, 101325.0), T(1499.9999999999998, 1500.0)
building face restriction: start
building face restriction: done
building face restriction: start
building face restriction: done
building face restriction: start
building face restriction: done
/Users/mtcampbe/CEESD/devel/omicron/production/emirge/grudge/grudge/trace_pair.py:505: UserWarning: Encountered unknown symbolic tag '<class 'mirgecom.euler._EulerCVTag'>', assigning a value of '261176863'. This is a temporary workaround, please ensure that tags are sufficiently distinct for your use case.
  warn("Encountered unknown symbolic tag "
building face restriction: start
building face restriction: done
/Users/mtcampbe/CEESD/devel/omicron/production/emirge/grudge/grudge/trace_pair.py:505: UserWarning: Encountered unknown symbolic tag '<class 'mirgecom.euler._EulerCVTag'>', assigning a value of '53455899'. This is a temporary workaround, please ensure that tags are sufficiently distinct for your use case.
  warn("Encountered unknown symbolic tag "

Example hangs at this point.

[inducer]

Ah right. hash() is not consistent across interpreter instances. pytools.persistent_dict has a whole separate hash infrastructure that solves this.

[inducer]

@MTCam As a workaround for the workaround, if you set the env var PYTHONHASHSEED=0, that'll prop this up until we fix this up.

https://docs.python.org/3/using/cmdline.html#envvar-PYTHONHASHSEED

[MTCam]

@MTCam As a workaround for the workaround, if you set the env var PYTHONHASHSEED=0, that'll prop this up until we fix this up.

docs.python.org/3/using/cmdline.html#envvar-PYTHONHASHSEED

Hrm. I tried this, and after it failed, I tried it like this:
mpiexec -n 2 env PYTHONHASHSEED=0 python -m mpi4py autoignition-mpi.py,
and the tag hash was still rank-disparate, resulting in a hang.

[inducer]

Just pushed a version that uses the persistent hash.

[MTCam]

Just pushed a version that uses the persistent hash.

Eureka! Thanks!

[MTCam]

Just FYI, this is totally OK - since it just requires the MIRGE-Com examples (and production drivers) to be updated, but we still get this error when running eager in parallel:

  File "/Users/mtcampbe/CEESD/devel/omicron/production/emirge/grudge/grudge/trace_pair.py", line 307, in interior_trace_pairs
    + cross_rank_trace_pairs(dcoll, vec, comm_tag=comm_tag)
  File "/Users/mtcampbe/CEESD/devel/omicron/production/emirge/grudge/grudge/trace_pair.py", line 508, in cross_rank_trace_pairs
    tag_ub = actx.mpi_communicator.Get_attr(MPI.TAG_UB)
AttributeError: 'PyOpenCLArrayContext' object has no attribute 'mpi_communicator'
Abort(1) on node 0 (rank 0 in comm 0): application called MPI_Abort(MPI_COMM_WORLD, 1) - process 0

The fix is to make sure to update everything to use the new API for getting the reasonable array context instead of just picking PyOpenCLArrayContext.

[inducer]

The fix is to make sure to update everything to use the new API for getting the reasonable array context instead of just picking PyOpenCLArrayContext.

This could have been done in a backward-compatible manner, and I'm really kicking myself that it wasn't, for no good reason other than perhaps that I was rushing to get it done. Generally, the change is what we want (that the array context is the source of truth for the communicator, rather than the discretization). Is there a point now to retrofitting backward compat now, or not really?

[MTCam]

The fix is to make sure to update everything to use the new API for getting the reasonable array context instead of just picking PyOpenCLArrayContext.

This could have been done in a backward-compatible manner, and I'm really kicking myself that it wasn't, for no good reason other than perhaps that I was rushing to get it done. Generally, the change is what we want (that the array context is the source of truth for the communicator, rather than the discretization). Is there a point now to retrofitting backward compat now, or not really?

Actually, in many ways this is working out better. This would have just added to our pile of technical debt if we were allowed to limp forward without making the changes. All is well imo. I warned folks not to be pulling new toolchains until we get all this worked out. I understand the grumblings too, but on some level we need to be fluid in supporting important changes coming down from upstream pkgs.