rccl --mca pml option fix when openmpi with ucx is used for rccl-test

[ahskabir]

Motivation

CVS's determine_mpi_pml_config() function fails to add --mca pml ucx to the mpirun command when mpi_pml is set to "ucx" in the config. Instead, it only passes -x UCX_TLS=auto (an invalid transport name) and -x UCX_NET_DEVICES= (empty). Without explicit PML=ucx, MPI falls back to a non-UCX transport that causes GPU runlist oversubscription and 2-3x bandwidth degradation.

Root Cause
In cvs/lib/rccl_lib.py, the determine_mpi_pml_config() function:
When mpi_pml="ucx": sets pml_param = "" (empty) instead of "--mca pml ucx"
Passes -x UCX_TLS=auto which is not a valid UCX transport name
Passes -x UCX_NET_DEVICES= (empty) which tells UCX to use no devices

The function only ever outputs --mca pml ob1 or empty string — the code path for --mca pml ucx does not exist.
According to OpenMPI 5.0 documentation, UCX PML should auto-select when InfiniBand/RoCE is detected — --mca pml ucx should not be required. However, CVS's invalid UCX environment variables (-x UCX_TLS=auto and -x UCX_NET_DEVICES=) cause UCX to fail initialization before OpenMPI can auto-detect it. With UCX broken, OpenMPI falls back to a non-UCX transport, which causes the GPU queue oversubscription and bandwidth degradation.

The OMPI_MCA_pml=ucx workaround forces UCX PML selection regardless of auto-detection, and UCX_NET_DEVICES=all fixes the broken device config so UCX can actually initialize. Both are needed because CVS actively passes bad values that override what would otherwise work automatically.

Technical Details

Following params are added in rccl_config.json file :
"ucx_tls": "rc,self,sm,tcp",
"_comment_ucx_tls": "When user requested UCX either leave this parameter value blank for auto assignment or assign value tcp",
"net_dev_list": "",
"_comment_net_dev_list": "Leave empty for auto-detection from backend NICs, or set explicitly e.g. ens26np0,ens27np0"

After the fix, in the rccl_config.json file user can provide mpi_pml: ucx and it correctly passes the args to mpirun

Test Plan

Testing was done by Ahsan Kabir and Ryan Lukasik separately, Ryan being the reported. I tested this bugfix first and tried many different combination of providing different type of pml like auto, ob1, ucx etc. For ucx, I built openmpi with ucx support. All of these combinations have been tried for for rccl_perf test. Same logic was added to rccl_regression.

Test Result

Testing summary is described in the comments section of https://amd-hub.atlassian.net/browse/AIMVT-248 and https://amd.atlassian.net/browse/DCCS-6464

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

[speriaswamy-amd]

The core fix is working. I tested this commit on two Ruby nodes with 16 ranks and exercised explicit ucx, explicit ob1, auto selecting UCX, a controlled auto fallback to ob1, and the duplicate rccl_regression path. All paths completed with zero #wrong values; the focused 16 MiB all-reduce runs were approximately 156–157 GB/s. I also smoke-tested all 12 available rccl-tests collectives. Ruff, the 18 existing unit tests, CI, and the docs build pass.

I am requesting one small robustness change before merge: explicit ob1 currently performs UCX NIC discovery before the PML decision. Ruby has a complete mapping, so its hardware run passes, but empty and partial mapping probes fail before ob1 can launch. Please skip discovery for explicit ob1 and make discovery failures descriptive. I have also left a should-fix comment for the checked-in configuration conflict.

[speriaswamy-amd]

Could we skip UCX NIC discovery when mpi_pml is explicitly ob1? This runs before the PML decision even though the ob1 branch discards all UCX parameters, making an existing ob1 run newly dependent on rocm-smi, rdma, lshw, and a complete GPU-to-NIC map.

I reproduced IndexError with an empty mapping and KeyError: rdma_dev with a partial mapping. Ruby passed because its mapping is complete, but the explicit-ob1 log confirms this unnecessary discovery still runs. The same guard is needed in rccl_regression around line 656.

A minimal change would be:

net_dev_list = mpi_params.get("net_dev_list", "")
if mpi_pml.lower() != "ob1" and not net_dev_list:
    net_dev_list = linux_utils.get_ucx_net_devices(phdl)

Please add a focused test that explicit ob1 does not call get_ucx_net_devices() in either execution path.

[ahskabir]

addressed as per chat discussion

[speriaswamy-amd]

Please validate the topology result before indexing it. get_gpu_nic_mapping_dict() can return an empty dict, and a card can lack rdma_dev when its PCI-bus match does not succeed. These currently escape as IndexError and KeyError, which do not explain what topology data is missing.

Could this raise a clear ValueError for an empty map or an unmapped card? It should also reject an empty final device list so the caller cannot construct -x UCX_NET_DEVICES=. For example, use next(iter(out_dict)) only after an explicit empty check and access each device with mapping.get("rdma_dev").

[ahskabir]

The method is updated and will be seen in the patch when I push it. It addresses all your review comments. Additionally, the get_gpu_nic_mapping_dict has some bugs related to using unguarded match from regex. I will create a bugfix ticket to fix it.

diff --git a/cvs/lib/linux_utils.py b/cvs/lib/linux_utils.py
index ef4dc7db..2396d1ff 100644
--- a/cvs/lib/linux_utils.py
+++ b/cvs/lib/linux_utils.py
@@ -863,27 +863,53 @@ def get_gpu_numa_dict(phdl):
 def get_ucx_net_devices(phdl):
     """
     Build UCX_NET_DEVICES string from backend NIC RDMA device names.
-
     Uses get_gpu_nic_mapping_dict() which maps each GPU card to its nearest
     backend NIC. The 'rdma_dev' key gives the IB/RDMA device name (e.g. bnxt_re0).
     UCX_NET_DEVICES requires IB device names with port suffix ':1'
     (e.g. bnxt_re0:1), NOT ethernet names (ens20np0).

+    Raises:
+        ValueError: if the GPU-NIC topology map is empty, if any card is
+            missing an rdma_dev mapping, or if the resulting device list
+            would be empty.
+
     Returns:
         str: Comma-separated UCX_NET_DEVICES string,
              e.g. 'bnxt_re0:1,bnxt_re1:1,bnxt_re2:1,...'
     """
     out_dict = get_gpu_nic_mapping_dict(phdl)
+    if not out_dict:
+        raise ValueError(
+            'get_gpu_nic_mapping_dict() returned an empty topology map; '
+            'cannot determine UCX_NET_DEVICES (no nodes found).'
+        )

     # Use node_0 as representative — NFS/homogeneous cluster, all nodes identical
-    node_0 = list(out_dict.keys())[0]
-    card_list = list(out_dict[node_0].keys())
+    node_0 = next(iter(out_dict))
+    card_dict = out_dict[node_0]
+    if not card_dict:
+        raise ValueError(
+            f'No GPU cards found for node {node_0!r} in the topology map; '
+            'cannot determine UCX_NET_DEVICES.'
+        )

     ucx_net_devices_list = []
-    for card_no in card_list:
-        rdma_dev = out_dict[node_0][card_no]['rdma_dev']  # e.g. 'bnxt_re0'
+    for card_no, mapping in card_dict.items():
+        rdma_dev = mapping.get('rdma_dev')
+        if not rdma_dev:
+            raise ValueError(
+                f'Card {card_no!r} on node {node_0!r} has no rdma_dev mapping '
+                '(PCI-bus match to a backend NIC failed); cannot determine '
+                'UCX_NET_DEVICES.'
+            )
         ucx_net_devices_list.append(f'{rdma_dev}:1')  # UCX needs port suffix :1

+    if not ucx_net_devices_list:
+        raise ValueError(
+            'Resolved zero UCX net devices; refusing to build an empty '
+            'UCX_NET_DEVICES value.'
+        )
+
     ucx_net_devices = ','.join(ucx_net_devices_list)
     log.info(f'Auto-detected UCX_NET_DEVICES: {ucx_net_devices}')
     return ucx_net_devices

[speriaswamy-amd]

Should fix: these comments contradict the values and documentation added by this PR. This line recommends tcp, although the new docstring says tcp alone can fail PML initialization; the device comment below uses ethernet names although discovery produces RDMA device:port names. Please recommend rc,self,sm,tcp and use an example such as bnxt_re0:1,bnxt_re1:1.

There is also an ownership conflict with the checked-in thor2_env_script.sh and cx7_env_script.sh: both export UCX_TLS=tcp, and Thor2 replaces the RDMA list with ethernet names. Because the payload sources the env script inside the launched shell, those later exports replace the values inherited from mpirun -x. Please either remove the conflicting template exports or explicitly reapply the computed UCX values after sourcing the script.

[ahskabir]

The comment for ucx_tls has been updated and will be seen in the patch that you will be pushed. As for the changes in thor2_env_script.sh, it was decided that we would leave it as-is since this PR is about fixing a specific bug and not meant to redesign the configurations.

diff --git a/cvs/input/config_file/rccl/rccl_config.json b/cvs/input/config_file/rccl/rccl_config.json
index d4fd35d3..6c7c06b4 100644
--- a/cvs/input/config_file/rccl/rccl_config.json
+++ b/cvs/input/config_file/rccl/rccl_config.json
@@ -8,7 +8,7 @@
             "mpi_dir": "/home/{user-id}/openmpi/bin",
             "mpi_oob_port": "eth0",
             "ucx_tls": "rc,self,sm,tcp",
-            "_comment_ucx_tls": "When user requested UCX either leave this parameter value blank for auto assignment or assign value tcp",
+            "_comment_ucx_tls": "When user requested UCX either leave this parameter value blank for auto assignment or assign values e.g. rc,self,sm,tcp..",
             "net_dev_list": "",
             "_comment_net_dev_list": "Leave empty for auto-detection from backend NICs, or set explicitly e.g. ens26np0,ens27np0"

[speriaswamy-amd]

Previously if there was a typo in config such as "mpi_pml": "uxc" , we produce a warning and move to auto detection, but now we got rid of the warning, behavior is still the same, but the warning might be useful

[ahskabir]

addressed as per chat discussion

[speriaswamy-amd]

We got rid of UCX_UNIFIED_MODE=y I thought this was required to tell UCX that all participating nodes have the same interface

[ahskabir]

restored.