Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set cuda devices for no-docker mode #1925

Merged
merged 6 commits into from
May 5, 2023

Conversation

swsuggs
Copy link
Contributor

@swsuggs swsuggs commented May 4, 2023

I believe this retains the existing behavior when using docker.
For no-docker mode, this sets CUDA_VISIBLE_DEVICES to the list of gpus, if specified, or to -1 if the gpu is not to be used.
It still defaults to using all gpus if use_gpu is True but no gpus are specified.

@christopherwoodall @jprokos26 Do you have any comments about this approach? Any cases I'm overlooking?

Should fix #1923 and #1902. Does not address #1922.

@swsuggs swsuggs requested a review from jprokos26 May 4, 2023 14:46
@dxoigmn
Copy link

dxoigmn commented May 4, 2023

We use CUDA_VISIBLE_DEVICES to control what GPUs a process can see and thus run on. It would be good if Armory respects an already set value of CUDA_VISIBLE_DEVICES in no-docker mode. We could switch to using Armory's flags, but CUDA_VISIBLE_DEVICES works regardless of process (assuming such processes do not muck with that environment variable).

@swsuggs
Copy link
Contributor Author

swsuggs commented May 4, 2023

@dxoigmn thanks for the input, let me know if this revision will satisfy your need.

@dxoigmn
Copy link

dxoigmn commented May 4, 2023

@dxoigmn thanks for the input, let me know if this revision will satisfy your need.

LGTM!

Copy link
Contributor

@jprokos26 jprokos26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will approve once addressed.

armory/eval/evaluator.py Show resolved Hide resolved
Copy link
Contributor

@jprokos26 jprokos26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM w/ minor formatting change

armory/eval/evaluator.py Outdated Show resolved Hide resolved
@swsuggs swsuggs merged commit b4f41f0 into twosixlabs:develop May 5, 2023
@swsuggs swsuggs deleted the 1923-no-docker-gpus branch May 5, 2023 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants