Skip to content

If CUDA is not found and USE_CUDA!=0, report error and ask user to set USE_CUDA=0#24939

Closed
xuhdev wants to merge 3 commits intopytorch:masterfrom
xuhdev:force-use-cuda
Closed

If CUDA is not found and USE_CUDA!=0, report error and ask user to set USE_CUDA=0#24939
xuhdev wants to merge 3 commits intopytorch:masterfrom
xuhdev:force-use-cuda

Conversation

@xuhdev
Copy link
Collaborator

@xuhdev xuhdev commented Aug 20, 2019

Currently we emit a warning and set USE_CUDA=0 in our script by ourselves. Instead, we should report an error and let user do this explicitly, as whether CUDA is used or not would affect so much of PyTorch.

Best viewed with "hide whitespace changes".

Per request by @jithunnair-amd at #23884 (comment)

…t USE_CUDA=0

Currently we emit a warning and set USE_CUDA=0 in our script by
ourselves. Instead, we should report an error and let user do this
explicitly, as whether CUDA is used or not would affect so much of
PyTorch.

Per request by @jithunnair-amd
@ezyang
Copy link
Contributor

ezyang commented Aug 21, 2019

cc @soumith @gchanan

@ezyang
Copy link
Contributor

ezyang commented Aug 21, 2019

@pytorchbot rebase this please

@gchanan
Copy link
Contributor

gchanan commented Aug 21, 2019

I don't have an opinion on this change, but I don't actually see anyone requesting this. The request linked, as I understand it, is about having a canonical variable that defines whether we are actually building with CUDA or not.

Edit: actually I think this is too user hostile, particularly when AFAICT no one is asking for it.

@ezyang
Copy link
Contributor

ezyang commented Aug 21, 2019

@gchanan To be clear, this PR is to unblock the AMD patch which doesn't work because the cmake variable is not set appropriately early enough. So there is a use case, but not from end users.

xuhdev added a commit to xuhdev/pytorch-builder that referenced this pull request Aug 21, 2019
xuhdev added a commit to xuhdev/pytorch-builder that referenced this pull request Aug 21, 2019
ezyang pushed a commit to pytorch/builder that referenced this pull request Aug 22, 2019
@xuhdev
Copy link
Collaborator Author

xuhdev commented Aug 22, 2019

Since pytorch/builder#345 is resolved, any idea when that will take effect so we can run a retest?

@gchanan
Copy link
Contributor

gchanan commented Aug 22, 2019

@ezyang I understand, I believe that issue could be resolved in a different way.

@xuhdev
Copy link
Collaborator Author

xuhdev commented Aug 22, 2019

The alternative I can think of would be to run all CUDA detection code before depending options are defined. Given the current status of the CUDA detection code, this might be difficult.

@jithunnair-amd
Copy link
Collaborator

@gchanan I sense an impasse. Can you please provide some direction, so the RCCL upstreaming PR can move forward (it's waiting on this)?

@xuhdev
Copy link
Collaborator Author

xuhdev commented Aug 23, 2019

@pytorchbot rebase this please

@gchanan
Copy link
Contributor

gchanan commented Aug 26, 2019

CC @kostmo.

@jithunnair-amd: is my summary above incorrect?

The request linked, as I understand it, is about having a canonical variable that defines whether we are actually building with CUDA or not.

So, I'd suggest we have a canonical way of saying whether we are actually building with CUDA or not.

Now, I don't know all the rules we've defined for flags, it seems like in general we are using USE_X for both did the user request x and are we building with x? Do you know, @kostmo?

@jithunnair-amd
Copy link
Collaborator

@jithunnair-amd: is my summary above incorrect?

The request linked, as I understand it, is about having a canonical variable that defines whether we are actually building with CUDA or not.

That sounds correct to me.

@xuhdev
Copy link
Collaborator Author

xuhdev commented Aug 27, 2019

@gchanan I agree that we should be consistent regarding this point, but it doesn't seem to be consistent. For example, currently USE_CUDA does not automatically turn off when the version number of USE_CUDA is too small, but if it is simply not found, then USE_CUDA is automatically turned off:

if(CUDA_VERSION VERSION_LESS 9.0)
message(FATAL_ERROR "PyTorch requires CUDA 9.0 and above.")
endif()

@gchanan
Copy link
Contributor

gchanan commented Aug 29, 2019

idk, that seems like a plausible choice.

@xuhdev
Copy link
Collaborator Author

xuhdev commented Sep 10, 2019

Closing now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: build Build system issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants