Skip to content

Conversation

cenit
Copy link
Contributor

@cenit cenit commented Aug 3, 2019

No description provided.

@cenit
Copy link
Contributor Author

cenit commented Aug 3, 2019

This will be very tricky to test in CI. I am interested in any kind of feedback!

@cenit cenit marked this pull request as ready for review August 3, 2019 22:12
[cuda,cudnn] add a message to make it clear their versions must be kept in sync
@cbezault
Copy link
Contributor

cbezault commented Aug 6, 2019

@cenit, is there any rush on this PR? @Rastaban is out of office right now and he's the one most familiar with updating the software on all the CI machines (since we need to get CUDA 10).

@cenit
Copy link
Contributor Author

cenit commented Aug 6, 2019

There is no Cuda installed in any CI machine, unfortunately. That's why it's so difficult to test this PR, since it starts from a broken port for the CI point of view. It works locally if you try (don't need a CUDA card to install CUDA software, if you know how to proceed ;) )

No rush for it anyway, but it might be nice if we ever get Cuda installed for future CI tests :)
(I have some scripts for headless installation on linux/windows if you need them)

@cbezault
Copy link
Contributor

cbezault commented Aug 6, 2019

I think we'd happily take those scripts. We definitely want our CI systems to be capable of building all the ports.

@cbezault cbezault self-assigned this Aug 14, 2019
@cenit
Copy link
Contributor Author

cenit commented Aug 21, 2019

@cbezault here it is. Taken directly from the darknet CI infrastructure

install_cuda.cmd.txt
install_cuda.sh.txt

@cenit
Copy link
Contributor Author

cenit commented Aug 24, 2019

@vicroms while integrating cudnn into some work, I found some bugs in OpenCV (3 and 4) dealt directly for my simplicity inside this PR.

@cenit
Copy link
Contributor Author

cenit commented Aug 28, 2019

@cbezault any news here?

@cbezault cbezault assigned Rastaban and unassigned cbezault Aug 28, 2019
@Rastaban
Copy link
Contributor

I am going to start bringing up a new set of VMs to update some other things, I will add this to the script while I am at it. It may not be until next week sometime that they are available though.

@cenit
Copy link
Contributor Author

cenit commented Aug 31, 2019

Ok! I hope this pr can be merged next next week!
Thanks for your work!

@cenit
Copy link
Contributor Author

cenit commented Sep 6, 2019

@Rastaban sorry for pinging again. News?

@Rastaban
Copy link
Contributor

Sorry to leave you hanging, I have not had a chance to bring up the new CI machines yet.

@cenit
Copy link
Contributor Author

cenit commented Sep 12, 2019

ok, do you have any estimation of timing?

@cenit
Copy link
Contributor Author

cenit commented Sep 14, 2019

@vicroms while integrating cudnn into some work, I found some bugs in OpenCV (3 and 4) dealt directly for my simplicity inside this PR.

since this is still true, I also added the fix to issue #7981

@Rastaban
Copy link
Contributor

cuda is now installed on the windows VMs.

@cenit
Copy link
Contributor Author

cenit commented Sep 18, 2019

wonderful. I merged with master to trigger a rebuild. I hope it will be a green check :)

@Rastaban
Copy link
Contributor

It looks like cuda is passing on all windows machines. Based on the warning messages in cudnn it should probably build for x64-windows-static, let me know if there are additional changes that are needed on the CI machine to get that passing.

I just installed cuda on the Linux CI machines and have started a rebuild on the vcpkg-Linux-PR pipeline. It may fail due to #8247, I will watch for the results.

@cenit
Copy link
Contributor Author

cenit commented Sep 19, 2019

if you prepared the environment as requested by the port file, it should work on linux x64 (I use it locally).
I will analyse the x64-windows-static now

@cenit
Copy link
Contributor Author

cenit commented Sep 19, 2019

@Rastaban x64-windows-static is an expected failure unfortunately.
Since cudnn is a dynamic-linking-only library, Vcpkg just exits saying Refusing to build unexpected dynamic library against the static CRT. (you have to request explicit tolerance for this situation in a custom triplet, I learned that with @vicroms when we were working on removing all references to CMAKE_EXPORT_ALL_SYMBOLS #5937 (comment) )

@cenit
Copy link
Contributor Author

cenit commented Sep 23, 2019

@Rastaban is there anything I can do to help deciding if merge this PR?

@Rastaban Rastaban merged commit 3532a7c into microsoft:master Sep 24, 2019
@cenit cenit deleted the dev/cenit/cudnn branch September 25, 2019 18:01
@ChaoJia
Copy link
Contributor

ChaoJia commented Oct 5, 2019

@cenit thanks for adding cudnn port. However, I think the way cuda port was implemented is better, as long as cudnn lib is put in the same directory as cuda (should be easy: they have the same directory structure). I've made one here. It's less cumbersome and works fine on Windows 10 so far.
By the way, in darknet CONTROL file, feature cudnn should also depend on package cudnn.

@cenit
Copy link
Contributor Author

cenit commented Oct 5, 2019

@ChaoJia I agree that a cudnn dependency is missing from darknet (I don’t know how it happened, it was the main reason of that pr... :D )
I don’t understand what’s the problem with CUDNN installed inside vcpkg folder. It works, at least in my cases. If possible, I’d like to be able to install CUDA also with vcpkg, not just checking if it is installed. Did you experience any problem? Why do you think this way is not good?

@ChaoJia
Copy link
Contributor

ChaoJia commented Oct 5, 2019

@cenit the main reason is that it is cumbersome. cudnn64_7.dll is 401 MB. If you have multiple projects using cudnn, vcpkg will make multiple copies of it. That could add up quickly.
cudnn is not built by vcpkg, so it does not come with a debug version and release version, while other non-empty packages do. The same goes for cuda.
Some projects might rely on cuda but not necessarily use vcpkg. If cuda is physically installed within vcpkg instead of the place where it used to be, CMake (and maybe some other software which rely on something provided by cuda) would probably not be able to find cuda naturally without any tweaks. In that case, cuda has to be installed twice. That's about 2 GB.
If cuda port remains the way it is right now, then it is advantageous to put cudnn in the cuda install path and keep an empty vcpkg port for cudnn too (at least path has already been set). No multiple copies are needed. Since cuda installation is taken care of by user, it is kind of straightforward to leave cudnn to user too.
Anyway, it's just a matter of personal taste, but please leave cuda port the way it is if it does not irritate you too much.

@cenit
Copy link
Contributor Author

cenit commented Oct 5, 2019

of course CUDA will remain like it is now, most of all because the user setup inside vcpkg is impossible.
Understood your point about cudnn

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.

5 participants