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

Adding row/column tests and benchmarks along with a benchmark/test building framework #331

Merged

Conversation

hyperbolic2346
Copy link
Collaborator

This PR adds c++ tests and benchmarks to spark-rapids-jni. The tests and benchmarks link in the helpers from cudf, so a test can be lifted directly from cudf or written in the same style as a cudf test. The benchmarks only support nvbench, as that is the preferred benchmarking tool.

To build with tests, pass -DBUILD_TESTS to the command line for the build and to build benchmarks, pass -DBUILD_BENCHMARKS. The test library used from cudf is built any time cudf is built, so no changes are necessary for that build. The benchmark library requires that benchmarks are built, so this is plumbed down through the pom.xml and build-libcudf.xml if the user selects benchmark building.

The change includes a new script build/run-in-docker that will run an executable in the same container environment that was used to build spark-rapids-jni. This script also allows an empty argument list and will drop the user into an interactive shell inside the container. This isn't a requirement, but ensures the same environment in the case the user is unable or unwilling to match the build environment.

I had to copy the 22.08 version of row_conversion.cu so the tests would properly work, but it is not quite ready to move to this repository yet.

Adding automatic running of tests after build was suggested as a feature, but left for another PR. I have added issue #330 to track it.

@hyperbolic2346 hyperbolic2346 self-assigned this Jun 16, 2022
@hyperbolic2346 hyperbolic2346 added the enhancement New feature or request label Jun 16, 2022
Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

The contributing guide needs to be updated with instructions on how to use the new build features.

build-libcudf.xml Show resolved Hide resolved
Copy link
Collaborator

@gerashegalov gerashegalov left a comment

Choose a reason for hiding this comment

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

LGTM

gerashegalov
gerashegalov previously approved these changes Jun 17, 2022
jlowe
jlowe previously approved these changes Jun 21, 2022
@jlowe
Copy link
Member

jlowe commented Jun 21, 2022

build

@gerashegalov
Copy link
Collaborator

Build needs more work

[2022-06-21T21:34:40.221Z] [INFO]      [exec] gmake[2]: *** No rule to make target 'libcudf.so', needed by 'libcudfjni.so'.  Stop.
[2022-06-21T21:34:40.221Z] [INFO]      [exec] gmake[2]: *** Waiting for unfinished jobs....
[2022-06-21T21:34:40.221Z] [INFO]      [exec] gmake[1]: *** [CMakeFiles/Makefile2:130: CMakeFiles/cudfjnistub.dir/all] Error 2

@hyperbolic2346
Copy link
Collaborator Author

Yes, this turned out to be a bug in the CMake Makefile Generator. I have a workaround and will update with it soon.

@hyperbolic2346
Copy link
Collaborator Author

build

@hyperbolic2346
Copy link
Collaborator Author

The workaround isn't working so well. Talking with Robert Maynard about it.

@hyperbolic2346
Copy link
Collaborator Author

build

@hyperbolic2346 hyperbolic2346 merged commit b78154d into NVIDIA:branch-22.08 Jun 24, 2022
@hyperbolic2346 hyperbolic2346 deleted the mwilson/tests_and_benchmarks branch June 24, 2022 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants