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

[flashlight-cuda] New Port #14676

Merged
merged 3 commits into from
Nov 30, 2020
Merged

[flashlight-cuda] New Port #14676

merged 3 commits into from
Nov 30, 2020

Conversation

jacobkahn
Copy link
Contributor

@jacobkahn jacobkahn commented Nov 20, 2020

Adds a new port for flashlight, an ML library written in C++. I'm one of the project authors.

A few things to note:

  • This port only contains support for the CUDA backend in flashlight. OpenCL/CPU backend support will be added in a separate port later when they are complete.
  • The library depends on NCCL for which there doesn't currently exist a port (I've just created a PR, ideally, we commit that port first) - it uses find_package natively with a FindNCCL.cmake module. NCCL and cuDNN are very similar in their installations next to CUDA, so the NCCL port works exactly like the vcpkg cuDNN port.
    • CI will probably fail until the NCCL port is committed
  • The features in the port depend on other features of the port:
lib
 ^
 |
 fl <— imgclass
 ^
 |
asr
  • Which triplets are supported/not supported? Have you updated the CI baseline?

Supports linux for now. I've tested x64-linux. The port has !(windows|osx), but I will test the port on macOS shortly. I don't expect Windows integration to be complete for some time, but I will update the port if/when it is.

Yes.

cc @tlikhomanenko @vineelpratap @xuqiantong @avidov @andresy @syhw @padentomasello

@NancyLi1013 NancyLi1013 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Nov 23, 2020
Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Can you revert changes about nccl and depends on #14683?

@jacobkahn
Copy link
Contributor Author

@JackBoosY done — I'll rebase on top of the NCCL port once it's done though.

@JackBoosY JackBoosY added depends:different-pr This PR or Issue depends on a PR which has been filed and removed requires:author-response labels Nov 24, 2020
@jacobkahn
Copy link
Contributor Author

@JackBoosY how can I get detailed logs of this failure in CI?

@JackBoosY
Copy link
Contributor

@jacobkahn
Copy link
Contributor Author

@JackBoosY I have permission. How do I get to this page for future CI runs?

@JackBoosY
Copy link
Contributor

@jacobkahn
Details-> View more details on Azure Pipelines -> * artifacts produced

@strega-nil Maybe we need to write documentation to the contributors.

@jacobkahn
Copy link
Contributor Author

@JackBoosY is there some way I can know what command CI ran when this error happened? I'm unable to reproduce it.

@JackBoosY
Copy link
Contributor

JackBoosY commented Nov 30, 2020

@jacobkahn Maybe the environment of our CI machine is different from yours. You can only use the log content to solve the regression.
And if you need more details of the regression, I will manually test it on the same machine and provide to you.

@jacobkahn
Copy link
Contributor Author

@JackBoosY fixed the issue. Needed a default feature.

@strega-nil strega-nil merged commit b7f03b4 into microsoft:master Nov 30, 2020
@strega-nil
Copy link
Contributor

Thanks @jacobkahn

@jacobkahn jacobkahn deleted the flashlight branch November 30, 2020 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! depends:different-pr This PR or Issue depends on a PR which has been filed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants