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

cuda/cl tags should be opt-in #64

Open
willscott opened this issue May 20, 2017 · 5 comments
Open

cuda/cl tags should be opt-in #64

willscott opened this issue May 20, 2017 · 5 comments
Assignees

Comments

@willscott
Copy link
Member

having dealt with this for a bit now, i've decided i care more, and that the optional libraries need to be opt-in parts of the build.

editors / standard ecosystem tools expect a 'go build' without tags to succeed, and tags are not always available. (e.g. atom doesn't have a way to set them https://www.bountysource.com/issues/42210626-does-go-plus-support-build-tags)

If i can't run go build successfully in the project because it fails on cuda headers that i can't install / test because my computer doesn't support them, i'm just not going to run the tests.

@ryscheng
Copy link
Member

To summarize my reason for going with opt-out: The high level goal is to be running as many of the tests as possible. It seems way too easy to make a bunch of opt-in build tags that we just end up forgetting to build or test. There's an example of how we opt-out of CUDA/CL tests in the Travis configuration.

I'd be happy to support this issue if we get a CI environment that can run all the tests, so that this concern is alleviated. I've opened this issue to track:
#67

In the meantime, you can easily setup your own test scripts in atom:
https://stackoverflow.com/questions/39146511/how-to-setup-a-custom-keybinding-to-run-a-script-or-execute-a-command-in-atom-ed

@willscott
Copy link
Member Author

test scripts don't solve this: the 'go plus' environment is what hooks in to the editor with inline coverage, refactoring, and error flagging. none of those happen if the build fails because cuda headers are missing.

@ryscheng
Copy link
Member

Okay, I see the problem. Seems a little silly for go-plus to not support custom build tags.
joefitzgerald/go-plus#573
joefitzgerald/go-plus#603

In my opinion, whether it's opt-out or opt-in should be independent of whether this Atom package properly let's you configure your go environment.

@willscott
Copy link
Member Author

i'm sympathetic to wanting to make sure that code stays alive and gets tested. at the same time, it feels like it's hopefully already protected by a pretty static API that isolates it, and all of the development we're going from here on out doesn't change what that small kernel of GPU work is.

having that be an imposition on work on the client, frontend, coordinator, or distributing replicas across machines is gonna get old.

@willscott
Copy link
Member Author

noting for myself that this caused a6ffbc6

Atom didn't catch it because it was stuck on not finding cuda.h, and makefile failed as well because of tag errors.

If i run into this again i break down and try to fix the environment and isolate the optional dependencies.

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

No branches or pull requests

2 participants