fix: kdd tree knn example segfault #2925
Conversation
Alexandr-Solovev
requested review from
Alexsandruss and
samir-nasibli
as code owners
|
/intelci: run |
Alexandr-Solovev
requested review from
icfaust,
Vika-F and
david-cortes-intel
...daal/src/algorithms/k_nearest_neighbors/kdtree_knn_classification_train_dense_default_impl.i
Show resolved
Hide resolved
...daal/src/algorithms/k_nearest_neighbors/kdtree_knn_classification_train_dense_default_impl.i
Show resolved
Hide resolved
.../algorithms/k_nearest_neighbors/kdtree_knn_classification_predict_dense_default_batch_impl.i
Outdated
Show resolved
Hide resolved
.../algorithms/k_nearest_neighbors/kdtree_knn_classification_predict_dense_default_batch_impl.i
Show resolved
Hide resolved
| @@ -50,18 +50,32 @@ struct Math | |||
|
|
|||
| static fpType sPowx(fpType in, fpType in1) { return _impl<fpType, cpu>::sPowx(in, in1); } | |||
|
|
|||
| static fpType xsPowx(fpType in, fpType in1) { return _impl<fpType, cpu>::xsPowx(in, in1); } | |||
There was a problem hiding this comment.
Could you please leave some comments in this header about the differences between the functions starting with 's' and the functions starting with 'xs' and when should each be called.
| static void vExp(SizeType n, const double * in, double * out) | ||
| { | ||
| #pragma omp simd | ||
| for (SizeType i = 0; i < n; ++i) out[i] = exp(in[i]); | ||
| } | ||
|
|
||
| static void xvExp(SizeType n, const double * in, double * out) | ||
| { | ||
| for (SizeType i = 0; i < n; ++i) out[i] = exp(in[i]); |
There was a problem hiding this comment.
Functions like this would run faster when using multiple threads. MKL runs then multi-threaded as far as I can tell.
There was a problem hiding this comment.
yes, you are right, but I wanted to add the section about these functions usage in threader_for(multithread for), because in such case to avoid calling multithread function inside the multithread loop we must use xFunc(single thread)
| // Not implemented | ||
| static double sErfInv(double in) { return std::numeric_limits<double>::quiet_NaN(); } | ||
|
|
||
| // Not implemented | ||
| static double xsErfInv(double in) { return std::numeric_limits<double>::quiet_NaN(); } |
There was a problem hiding this comment.
I haven't looked at what exact compilation arguments go into the icx invocations, but isn't the default for it to have the equivalent of GCC's -ffast-math which implicitly assumes that NaNs and Infs will not be encountered?
If so, would this NaN be checked somewhere? Could it throw an exception instead?
And how about taking these functions from the cephes library from Netlib which is public domain?
|
Was anything else merged that would impact kd tree knn seg faults? Because these are no longer showing up in CI |
|
Description
Please include a summary of the change. For large or complex changes please include enough information to introduce your change and explain motivation for it.
Changes proposed in this pull request: