-
Notifications
You must be signed in to change notification settings - Fork 564
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
Promote trustworthiness_score
to public header, add missing includes, update dependencies
#3968
Promote trustworthiness_score
to public header, add missing includes, update dependencies
#3968
Conversation
trustworthiness_score
to public header, fix include paths, update dependenciestrustworthiness_score
to public header, add missing includes, update dependencies
…ude-paths-dependency-versions-and-metrics-header
…arge enough to hold the accumulated sums (NVIDIA/cub#201)
rerun tests (we just pushed/deployed some CI fixes) |
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 all the improvements! Approving, though the relevant CI jobs are showing the following doxygen issue (which is tricky to parse in the logs):
Generating docs for compound ML:/workspace/cpp/include/cuml/metrics/metrics.hpp:344: error: argument 'in' of command @param is not found in the argument list of ML::Metrics::trustworthiness_score(const raft::handle_t &h, const math_t *X, math_t *X_embedded, int n, int m, int d, int n_neighbors, int batchSize) (warning treated as error, aborting now)
We should be able to merge once that's fixed. Thanks!
NULL, cub_bytes, f_idx.data(), f_idx_sorted.data(), f_sorted.data(), | ||
f_sorted.data(), n_train, 0, 8 * sizeof(int), stream); | ||
NULL, cub_bytes, f_sorted.data(), f_sorted.data(), f_idx.data(), | ||
f_idx_sorted.data(), n_train, 0, 8 * sizeof(math_t), stream); |
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.
😿 |
…ude-paths-dependency-versions-and-metrics-header
Codecov Report
@@ Coverage Diff @@
## branch-21.08 #3968 +/- ##
===============================================
Coverage ? 85.32%
===============================================
Files ? 230
Lines ? 18095
Branches ? 0
===============================================
Hits ? 15439
Misses ? 2656
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
@gpucibot merge |
@@ -23,6 +23,10 @@ dependencies: | |||
- umap-learn | |||
- scikit-learn=0.23.1 | |||
- treelite=1.3.0 | |||
- statsmodels |
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.
What are these new dependencies for? Just curious.
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.
They're required in cuML Python tests, but only listed in the rapids-test-env
metapackage.
…s, update dependencies (rapidsai#3968) * Updates dask/distributed versions to match cuDF (rapidsai/cudf#8458) * Updates to Thrust v1.12.0 to align with cuDF and cuGraph * Don't include the src and src_prims directories in `cuml::cuml++` target's public include paths * Add missing `<cstddef>` and `<cstdint>` include directives * Promote `trustworthiness_score` to public `cuml/metrics/metrics.hpp` header and update Cython * Compile Cython with `-std=c++17` * Remove `-Wstrict-prototypes` Cython warning * Fixes linker error in debug builds * Fixes rapidsai#3885 Authors: - Paul Taylor (https://github.com/trxcllnt) Approvers: - Dante Gama Dessavre (https://github.com/dantegd) - AJ Schmidt (https://github.com/ajschmidt8) URL: rapidsai#3968
cuml::cuml++
target's public include paths<cstddef>
and<cstdint>
include directivestrustworthiness_score
to publiccuml/metrics/metrics.hpp
header and update Cython-std=c++17
-Wstrict-prototypes
Cython warning