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

Windows install fix (try #2) #47

Merged
merged 1 commit into from Dec 13, 2016
Merged

Windows install fix (try #2) #47

merged 1 commit into from Dec 13, 2016

Conversation

mrakgr
Copy link
Contributor

@mrakgr mrakgr commented Dec 2, 2016

As before, I've modified the file by changing CUDA_PATH to CUDA_PATH_V7_5 and pointed the library directories to the correct location. It might be worth checking out whether like on Windows, Linux also has CUDA_PATH_V7_5 before accepting this fix.

As before, I've modified the file by changing `CUDA_PATH` to `CUDA_PATH_V7_5` and pointed the library directories to the correct location. It might be worth checking out whether like on Windows, Linux also has `CUDA_PATH_V7_5` before accepting this fix.
@tmcdonell
Copy link
Owner

Thanks for the patch! Sorry for the super late reply; busy weekend and start of the week...

The linux boxes I have access to have neither CUDA_PATH nor CUDA_PATH_V7_5 set up by default. The way that the script is set up though is that CUDA_PATH is the first place it checks for your installation, followed by the other locations if that isn't found. So I don't think changing this to CUDA_PATH_V7_5 is a good idea, because (1) it locks the package a particular version of nvcc, and (2) that was meant to be a user-configurable option; e.g. bash> env CUDA_PATH=... cabal install. (although, I realise just now that I have not mentioned that in the instructions, so I will do that now...)

@mwu-tow does the change to library directories look right to you?

@mrakgr
Copy link
Contributor Author

mrakgr commented Dec 6, 2016

How about making CUDA_PATH_V7_5 the primary location and CUDA_PATH the secondary one then? It would make sense that the most up to date Cuda SDK would be the default one in case other packages decide to use CUDA_PATH as the default. On Windows, the Cuda SDK installer sets those variables automatically. I had no idea that in Linux one needed to do it by hand.

@mwu-tow
Copy link
Contributor

mwu-tow commented Dec 6, 2016

Preferring CUDA_PATH has the advantage that the user can explicitly decide which toolkit version should be used by setting the variable. While doing something like set CUDA_PATH=%CUDA_PATH_V7_5% looks sensible, requiring user to overwrite CUDA_PATH_V7_5 as a customization point doesn't feel right.

It will also mean that the toolkit version the was installed as the last will be used by default but I don't think it is a big problem. Also, the latest released version is 8, not 7.5.

The previous paths worked for me and my colleagues for quite a long time. I suspect that they must have been changed with Cuda 8 release. I can confirm that that Cuda 8 installation requires the proposed change. I'm not sure why your other Cuda installations (7.5 and below) also don't get recognized properly — perhaps Nvidia updated the older toolkits as well to follow the new structure?

At the moment I believe the best approach would be to check both locations and use the directory that exists.

@mrakgr
Copy link
Contributor Author

mrakgr commented Dec 7, 2016

perhaps Nvidia updated the older toolkits as well to follow the new structure?

I really do not understand this. It is unlikely that Nvidia would do this, and even more so as the Cuda 6.5 SDK which I installed over a year ago has the same directory structure.

@mwu-tow
Copy link
Contributor

mwu-tow commented Dec 7, 2016

You are right, it is extremely unlikely. Still, the previous paths worked for me on many machines with Cuda 7 and 7.5 and I quite can't understand why we end up with different structures of Cuda Toolkit.
Nvidia seems to different installers depending on a Windows version. Could this be a case? (though still it would be surprising if they used different structure for different Windows versions)

I'd confirm the paths on my older installations but currently I'm sick — I'll have a look at this when I get to my workplace. For now you'll have to believe me that I did not make these paths up. ;-)

@mwu-tow
Copy link
Contributor

mwu-tow commented Dec 12, 2016

I checked this, I was wrong here. The path in the Toolkit has always been lib\x64. What was changed however was Setup.hs — the regession comes from this commit:
3ba0614

where

getCudaLibraryPath :: CudaPath -> Platform -> FilePath
getCudaLibraryPath (CudaPath path) (Platform arch os) = path </> libSubpath
  where
    libSubpath = case os of
      Windows -> "lib" </> case arch of
         I386   -> "Win32"
         X86_64 -> "x64"
         _      -> error $ printf "Unexpected Windows architecture %s.\nPlease report this issue to https://github.com/tmcdonell/cuda/issues\n" (show arch)

      OSX       -> "lib"

      -- For now just treat all other systems similarly
      _ -> case arch of
         X86_64 -> "lib64"
         I386   -> "lib"
         _      -> "lib"  -- TODO: how should this be handled?

was replaced with

cudaLibraryPath (Platform arch os) installPath = installPath </> libpath
  where
    libpath =
      case (os, arch) of
        (Windows, I386)   -> "Win32"
        (Windows, X86_64) -> "x64"
        (OSX,     _)      -> "lib"    -- MacOS does not distinguish 32- vs. 64-bit paths
        (_,       X86_64) -> "lib64"  -- treat all others similarly
        _                 -> "lib"

The paths should be "lib" </> "Win32" and "lib" </> "x64", as @mrakgr observed. Sorry for the confusion.

@tmcdonell tmcdonell merged commit 6f89a4b into tmcdonell:master Dec 13, 2016
@tmcdonell
Copy link
Owner

Thanks guys! Sorry I messed up the windows library paths in 3ba0614 ):

By the way, what is the default install path for CUDA on windows? If we add that here, it will be automatically checked as well.

@mrakgr mrakgr deleted the patch-2 branch December 13, 2016 09:32
@mrakgr
Copy link
Contributor Author

mrakgr commented Dec 13, 2016

C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA. It has the folders v6.5,v7.0,v7.5,v8.0, so for Cuda 7.5 SDK the full path is C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v7.5. The reason I am so verbose is because I am not sure whether on Linux it separates the versions. Also do not forget to change the \s to /s.

tmcdonell added a commit to tmcdonell/cufft that referenced this pull request Dec 19, 2016
tmcdonell added a commit that referenced this pull request Jan 6, 2017
minor bug fix release

ping: #43
ping: #45
ping: #47
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.

None yet

3 participants