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] remove src/treelearner/kernels #6766

Merged
merged 3 commits into from
Jan 2, 2025

Conversation

shiyu1994
Copy link
Collaborator

Remove these files because they are used in old CUDA version, which is not supported anymore.

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

Nice catch!

Then the following variables are not needed anymore:

LightGBM/CMakeLists.txt

Lines 256 to 273 in 60b0155

set(
BASE_DEFINES
-DPOWER_FEATURE_WORKGROUPS=12
-DUSE_CONSTANT_BUF=0
)
set(
ALLFEATS_DEFINES
${BASE_DEFINES}
-DENABLE_ALL_FEATURES
)
set(
FULLDATA_DEFINES
${ALLFEATS_DEFINES}
-DIGNORE_INDICES
)
message(STATUS "ALLFEATS_DEFINES: ${ALLFEATS_DEFINES}")
message(STATUS "FULLDATA_DEFINES: ${FULLDATA_DEFINES}")

UPD: this code also can be removed:

LightGBM/CMakeLists.txt

Lines 638 to 644 in 60b0155

# histograms are list of object libraries. Linking object library to other
# object libraries only gets usage requirements, the linked objects won't be
# used. Thus we have to call target_link_libraries on final targets here.
if(BUILD_CLI)
target_link_libraries(lightgbm PRIVATE ${histograms})
endif()
target_link_libraries(_lightgbm PRIVATE ${histograms})

@shiyu1994 shiyu1994 requested a review from StrikerRUS December 25, 2024 07:37
Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@shiyu1994
Copy link
Collaborator Author

I just restarted the VM for CUDA CI agent. But it seems that the agent is not on yet. Let's wait a few hours to see if it works.

@jameslamb
Copy link
Collaborator

Thanks! I just restarted CUDA CI for this PR: https://github.com/microsoft/LightGBM/actions/runs/12490727657?pr=6766

@jameslamb
Copy link
Collaborator

@shiyu1994 looks like something else might need to be done. The job has been waiting almost 8 hours and hasn't been picked up by the runner.

Requested labels: self-hosted, linux
Job defined at: microsoft/LightGBM/.github/workflows/cuda.yml@refs/pull/6766/merge
Waiting for a runner to pick up this job...

Screenshot 2024-12-27 at 9 20 41 PM

build link: https://github.com/microsoft/LightGBM/actions/runs/12490727657/job/34924599078?pr=6766

I do see one runner at https://github.com/microsoft/LightGBM/actions/runners?tab=self-hosted ... does it matter that the label is "Linux" (capital L) and our configuration is looking for "linux" (lowercase l)?

runs-on: [self-hosted, linux]

Could you try changing the label there to lowercase "linux"? I don't have permission.

@jameslamb
Copy link
Collaborator

:/ I tested in another PR... don't think the case of the label is the problem: #6768 (comment)

I don't see any clearly-relevant reports at https://github.com/actions/runner/issues, and it looks like the last release there was about 6 weeks ago: https://github.com/actions/runner/releases

@shiyu1994 hopefully you can help restart the agent process or maybe update it to the newest release of https://github.com/actions/runner? I'm not sure what else we can do.

@shiyu1994
Copy link
Collaborator Author

:/ I tested in another PR... don't think the case of the label is the problem: #6768 (comment)

I don't see any clearly-relevant reports at https://github.com/actions/runner/issues, and it looks like the last release there was about 6 weeks ago: https://github.com/actions/runner/releases

@shiyu1994 hopefully you can help restart the agent process or maybe update it to the newest release of https://github.com/actions/runner? I'm not sure what else we can do.

I was on vacation during last week. Let me try.

@shiyu1994
Copy link
Collaborator Author

I just restarted the agent process. It seems work now.

@shiyu1994 shiyu1994 merged commit f3bd64a into master Jan 2, 2025
49 checks passed
@shiyu1994 shiyu1994 deleted the remove-kernels-for-old-cuda-version branch January 2, 2025 06:55
@jameslamb
Copy link
Collaborator

Great, thanks so much @shiyu1994 !!! I'll work on getting the other approved PRs updated and merged.

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

Successfully merging this pull request may close these issues.

3 participants