Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Build integrated Python package library #3144
Build integrated Python package library #3144
Changes from all commits
b1841bd
27dffcb
2958084
519a5b9
84b6804
ec4b3f4
1fc7878
8e66b4e
af176a6
e0c71cb
81e6773
4dac9f5
180c929
80318e0
1782aed
3013f4d
9d0614c
e567f33
d5fad62
dc5ef75
4fd05bc
8a23901
8f140dc
f654d91
2597549
db60d59
e274c8e
e4d2e3a
6acfd26
ee9f545
5d85aa1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please comment on this from compatibility side? Why 1.2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to be the
latestearliest stable version that contains all the required api's (and 2.x seems to have been a long experiment: https://www.extremetech.com/computing/309842-opencl-3-0-kicks-off-with-a-huge-step-backwards).But the library compiles without specifying the version (with a few extra messages), so we can remove this if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification! I'm afraid I have too little knowledge about OpenCL to take a responsibility for this change. I'd better leave it for your decision or more experienced in OpenCL maintainer.
I just want to make it possible that as many as possible users will be able to simply download our wheel and run LightGBM with GPU support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, including the 1.2 guard when you compile the headers prevents you from inadvertently including a call to a 2.x function, which would limit the library to newer implementations. OpenCL versions are backward compatible, so a 2.x implementation can run a 1.2 application/library. (See item 3 here for example: http://developer.amd.com/wordpress/media/2013/12/AMD_APP_SDK_FAQ1.pdf)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent! Thanks again!