[core][refactor] Move accelerator-specific environment variables to ray_constants.py to avoid redefining them

[kevin85421]

Why are these changes needed?

Env vars like CUDA_VISIBLE_DEVICES_ENV_VAR are defined in both ray_constants.py and nvidia_gpu.py. This PR moves related env vars to ray_constants.py to avoid redefining them.

Related issue number

Checks

• I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've added any new APIs to the API Reference. For example, if I added a
    method in Tune, I've added it in doc/source/tune/api/ under the
    corresponding .rst file.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

[kevin85421]

This is the same as ASCEND_RT_VISIBLE_DEVICES_ENV_VAR.

[edoakes]

Hm I think it's preferable to keep accelerator-specific logic isolated in the relevant accelerators/ file to avoid polluting the top-level Ray core namespace.

If we want to avoid redefining commonly-used ones, we can import them into ray_constants as well.

@jjyao WDYT?

[kevin85421]

Hm I think it's preferable to keep accelerator-specific logic isolated in the relevant accelerators/ file to avoid polluting the top-level Ray core namespace.

Note that some environment variables are used not only by xxxxxxpu.py but also by other files, which import them from ray_constants.py. For example, you can search CUDA_VISIBLE_DEVICES_ENV_VAR.

If we want to avoid redefining commonly-used ones, we can import them into ray_constants as well.

In my opinion, it's better not to import any Ray-related dependencies into ray_constants.py to avoid potential circular dependency issues in the future. We should aim to keep it as a top-level Python module in Ray.

[kevin85421]

Another way is to create accelerator_constants.py and it doesn't import any other Ray Python modules. All files want to use the env vars need to import the file.

[jjyao]

To avoid duplication, we can remove from ray_constants.py and just import the relevant accelerator file to get the corresponding constant?

[kevin85421]

This also makes sense. @edoakes does this make sense to you?

[edoakes]

yea this sounds better to me

[edoakes]

Nice! 🚀

[edoakes]

@justinvyu PTAL for codeowner approval, and also can you help me understand why we depend on this env var directly?

[justinvyu]

We need to basically overwrite the Ray Core behavior of restricting CUDA_VISIBLE_DEVICES=[ray_gpu_ids] (where ray_gpu_ids is a list of devices assigned to the actor) to instead set the env var to the set of ALL devices on a node that are used by training workers in the group. NCCL needs this in order to do cross device communication during training.

[justinvyu]

Thanks!

[justinvyu]

We need to basically overwrite the Ray Core behavior of restricting CUDA_VISIBLE_DEVICES=[ray_gpu_ids] (where ray_gpu_ids is a list of devices assigned to the actor) to instead set the env var to the set of ALL devices on a node that are used by training workers in the group. NCCL needs this in order to do cross device communication during training.