Add TESTING.md and BENCHMARKING.md #672
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
harrism
added
3 - Ready for Review
harrism
requested review from
thomcom and
isVoid
and removed request for
zhangjianting
This was referenced Sep 14, 2022
Merged
thomcom
reviewed
Sep 14, 2022
thomcom
requested changes
Sep 14, 2022
| reaches its saturation bottleneck, whether that bottleneck is bandwidth or computation. Using data | ||
| sets larger than this point is generally not helpful, except in specific cases where doing so | ||
| exercises different code and can therefore uncover regressions that smaller benchmarks will not | ||
| (this should be rare). |
There was a problem hiding this comment.
Do think we should suggest here that benchmarks should only be written for public APIs?
Co-authored-by: H. Thomson Comer <[email protected]>
Co-authored-by: H. Thomson Comer <[email protected]>
isVoid
reviewed
Sep 20, 2022
cpp/doc/BENCHMARKING.md
Outdated
Comment on lines
22
to
26
| In the interest of improving compile time, whenever possible, test source files should be `.cpp` | ||
| files because `nvcc` is slower than `gcc` in compiling host code. Note that `thrust::device_vector` | ||
| includes device code, and so must only be used in `.cu` files. `rmm::device_uvector`, | ||
| `rmm::device_buffer` and the various `column_wrapper` types described in [Testing](TESTING.md) | ||
| can be used in `.cpp` files, and are therefore preferred in test code over `thrust::device_vector`. |
There was a problem hiding this comment.
Does using NVBench precludes naming the file as cpp? NVBench header is cuh.
There was a problem hiding this comment.
Never thought about that. Asking.
There was a problem hiding this comment.
I think nvbench is .cu only...
thomcom
approved these changes
Sep 20, 2022
|
rerun tests |
isVoid
approved these changes
Sep 21, 2022
|
@gpucibot merge |
rapids-bot bot
pushed a commit
that referenced
this pull request
Sep 30, 2022
This PR mimics rapidsai/cudf#11475 to add libcuspatial's C++ developer guide to the rendered Doxygen HTML docs. See that PR for detailed discussion. Contributes to #598 . Closes #235 Note that this PR depends on #672 and #667 so the changed files will include changes from those PRs until they are merged. Authors: - Mark Harris (https://github.com/harrism) Approvers: - H. Thomson Comer (https://github.com/thomcom) - Vyas Ramasubramani (https://github.com/vyasr) - AJ Schmidt (https://github.com/ajschmidt8) - Michael Wang (https://github.com/isVoid) URL: #673
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Contributes to #598. Adds
cpp/doc/TESTING.mdandcpp/doc/BENCHMARKING.md, the remaining parts of the libcuspatial C++ developer documentation. These documents cover unit testing and unit benchmarking best practices, respectively.Checklist