Standardize UCX config separator to -

[pentschev]

Some of the UCX configurations use _ whereas others use -. This is confusing so we now standardize everything to -. This also fixes an inconsistency from #5526, where configuration files used create-cuda-context, but the configuration read was create_cuda_context.

[jrbourbeau]

Thanks @pentschev. In general I'm in favor of this change as it's more consistent with the naming convention of other configuration values. I am wondering if this makes a practical difference as my current understanding is our configuration system will treat - and _ the same. From https://docs.dask.org/en/stable/configuration.html#directly-within-python:

Note that the set function treats underscores and hyphens identically. For example, dask.config.set({'scheduler.work-stealing': True}) is equivalent to dask.config.set({'scheduler.work_stealing': True}).

[quasiben]

Agreed, it shouldn't make any functional difference but will be consistent throughout our codebase. @pentschev any more additional thoughts ?

[pentschev]

Thanks @pentschev. In general I'm in favor of this change as it's more consistent with the naming convention of other configuration values. I am wondering if this makes a practical difference as my current understanding is our configuration system will treat - and _ the same. From https://docs.dask.org/en/stable/configuration.html#directly-within-python:

Yes, and no. The reason I wanted to do this is because we may want to get the current/default configs with dask.config.get("distributed.comm.ucx") and rewrite them via the returned dict programatically, see what I'm doing in https://github.com/rapidsai/dask-cuda/pull/806/files#diff-fae45034757fc51999601c0f9886684027d56d2db243beb98ac08ef380731da1R301-R309 . However, before the PR I just linked to, we had the options hardcoded in https://github.com/rapidsai/dask-cuda/blob/branch-21.12/dask_cuda/utils.py#L299-L308, which would failed because of a missing recently introduced new option from Distributed (i.e., create_cuda_context). Therefore, this is mostly to avoid confusion when you dask.config.get and want to change configurations that are later passed to dask.config.{set,update}.

[pentschev]

To be clearer with an example, this is what we get before this PR:

In [1]: import dask, distributed

In [2]: dask.config.get("distributed.comm.ucx")
Out[2]:
{'cuda_copy': None,
 'tcp': None,
 'nvlink': None,
 'infiniband': None,
 'rdmacm': None,
 'net-devices': None,
 'reuse-endpoints': None,
 'create-cuda-context': None}

See how _ (in cuda_copy) and - (in create-cuda-context and others) are different in the returned dict above.

[pentschev]

I'm attempting to resolve the gpuCI test that failed in #5540 .

[pentschev]

rerun tests

[jrbourbeau]

Ah I see, thank you for the clarification. I've not had a chance to think too deeply about it yet, but I wonder if the dask.config.canonical_name utility might be useful for making how dask-cuda handles this case more flexible

[pentschev]

Ah I see, thank you for the clarification. I've not had a chance to think too deeply about it yet, but I wonder if the dask.config.canonical_name utility might be useful for making how dask-cuda handles this case more flexible

Thanks @jrbourbeau , I had missed that utility. I agree, it seems useful and I'll make sure to use it in Dask-CUDA to prevent the issue I mentioned. For now, I think we could merge this and make it consistent anyway, there should be no downside in doing so.

[jrbourbeau]

Yep, agreed -- thanks @pentschev

[pentschev]

Thanks @jrbourbeau and @quasiben !