Skip to content

Add gpu support for Rust binding #1925

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

Closed
wants to merge 11 commits into from

Conversation

getumen
Copy link

@getumen getumen commented Nov 19, 2021

Add enable_gpu_evaluation in Rust binding.

I attempted to run tests.
However I failed to runya make in catboost folder to make sure the code builds in step 2.

Before submitting a pull request, please do the following steps:

  1. Read instructions for contributors.
  2. Run ya make in catboost folder to make sure the code builds.
$ ./ya make -k
...
In file included from /home/yoshihiro/github/catboost/catboost/jvm-packages/catboost4j-prediction/src/native_impl/ai_catboost_CatBoostJNIImpl.cpp:1:
/home/yoshihiro/github/catboost/catboost/jvm-packages/catboost4j-prediction/src/native_impl/ai_catboost_CatBoostJNIImpl.h:4:10: fatal error: 'jni.h' file not found
#include <jni.h>  // Y_IGNORE
         ^~~~~~~
1 error generated.
...
  1. Add tests that test your change.
  2. Run tests using ya make -t -A command.
  3. If you haven't already, complete the CLA.

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

@getumen getumen changed the title Feature rust gpu support Add gpu support for Rust binding Nov 19, 2021
@getumen getumen marked this pull request as draft November 20, 2021 04:24
@andrey-khropov
Copy link
Member

@getumen

I'm sorry that this PR slipped through the cracks. Thank you for this PR, it is actually fine and we want to merge it.

We've recently switched from ya.make - based build to CMake-based one and error with JNI is no longer relevant. And to test Rust package it is sufficient to run usual cargo commands in rust-package subdirectory.

But supporting CUDA in Rust build will now require a change in call to build_native.py to add CUDA-relevant flags here:

let build_cmd_status = Command::new("../../../build/build_native.py")
.args(&[
"--targets", "catboostmodel",
"--build-root-dir", out_dir.to_str().unwrap(),
])
.status()
.unwrap_or_else(|e| {
panic!("Failed to run build_native.py : {}", e);
});
.

--have-cuda           Enable CUDA support
  --cuda-root-dir CUDA_ROOT_DIR
                        CUDA root dir (taken from CUDA_PATH or CUDA_ROOT by default)

--have-cuda will be required and maybe --cuda-root-dir should be also passed to override default CUDA path (but this is not strictly necessary).

@getumen getumen marked this pull request as ready for review April 13, 2023 00:13
@andrey-khropov
Copy link
Member

Our sync robot marks PR as 'closed' but in fact is has been merged.

@getumen getumen deleted the feature-rust-gpu-support branch April 17, 2023 05:09
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.

2 participants