[GitHub] [incubator-mxnet] ciyongch commented on a change in pull request #17318: Enable MKL-DNN FullyConnected backward
ciyongch commented on a change in pull request #17318: Enable MKL-DNN
FullyConnected backward
URL: https://github.com/apache/incubator-mxnet/pull/17318#discussion_r380416282
##
File path: tests/cpp/include/test_mkldnn.h
##
@@ -63,24 +63,24 @@ struct TestArrayShapes {
};
// Init arrays with the default layout.
-inline static void InitDefaultArray(NDArray *arr, bool is_rand = false) {
+inline static void InitDefaultArray(NDArray *arr, bool is_rand = false, int
max = 50) {
const TBlob = arr->data();
mshadow::default_real_t *data = blob.dptr();
int size = blob.Size();
for (int i = 0; i < size; i++)
if (is_rand) {
- data[i] = (std::rand() % 100) - 50;
+ data[i] = (std::rand() % (max * 2)) - max;
Review comment:
Ok, let's keep it as is for now.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ciyongch commented on a change in pull request #17318: Enable MKL-DNN FullyConnected backward
ciyongch commented on a change in pull request #17318: Enable MKL-DNN
FullyConnected backward
URL: https://github.com/apache/incubator-mxnet/pull/17318#discussion_r379962028
##
File path: tests/cpp/include/test_mkldnn.h
##
@@ -63,24 +63,24 @@ struct TestArrayShapes {
};
// Init arrays with the default layout.
-inline static void InitDefaultArray(NDArray *arr, bool is_rand = false) {
+inline static void InitDefaultArray(NDArray *arr, bool is_rand = false, int
max = 50) {
const TBlob = arr->data();
mshadow::default_real_t *data = blob.dptr();
int size = blob.Size();
for (int i = 0; i < size; i++)
if (is_rand) {
- data[i] = (std::rand() % 100) - 50;
+ data[i] = (std::rand() % (max * 2)) - max;
Review comment:
With `memcmp` to check the results, then the only choice is integer numbers.
Is it reasonable to check the results by `AssertEqual` within a small enough
threshold like 1e-6, then we can keep the floating number with better
distribution?
Or we can just increase `max` to filling more different numbers other than
only -1 and 0.
What do you think?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ciyongch commented on a change in pull request #17318: Enable MKL-DNN FullyConnected backward
ciyongch commented on a change in pull request #17318: Enable MKL-DNN
FullyConnected backward
URL: https://github.com/apache/incubator-mxnet/pull/17318#discussion_r379900144
##
File path: tests/cpp/include/test_mkldnn.h
##
@@ -63,24 +63,24 @@ struct TestArrayShapes {
};
// Init arrays with the default layout.
-inline static void InitDefaultArray(NDArray *arr, bool is_rand = false) {
+inline static void InitDefaultArray(NDArray *arr, bool is_rand = false, int
max = 50) {
const TBlob = arr->data();
mshadow::default_real_t *data = blob.dptr();
int size = blob.Size();
for (int i = 0; i < size; i++)
if (is_rand) {
- data[i] = (std::rand() % 100) - 50;
+ data[i] = (std::rand() % (max * 2)) - max;
Review comment:
I've no idea about why the range was set to [-50, 50) previously, and I
can't figure out any specific reasons to use this range for the tests (any
upper bound test?). It'll be great if you have any background for it.
But anyway, the tensors with only two values (-1 and 0, 50% are 0) might not
be a good candidate for the tests.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ciyongch commented on a change in pull request #17318: Enable MKL-DNN FullyConnected backward
ciyongch commented on a change in pull request #17318: Enable MKL-DNN
FullyConnected backward
URL: https://github.com/apache/incubator-mxnet/pull/17318#discussion_r379387996
##
File path: tests/cpp/include/test_mkldnn.h
##
@@ -63,24 +63,24 @@ struct TestArrayShapes {
};
// Init arrays with the default layout.
-inline static void InitDefaultArray(NDArray *arr, bool is_rand = false) {
+inline static void InitDefaultArray(NDArray *arr, bool is_rand = false, int
max = 50) {
const TBlob = arr->data();
mshadow::default_real_t *data = blob.dptr();
int size = blob.Size();
for (int i = 0; i < size; i++)
if (is_rand) {
- data[i] = (std::rand() % 100) - 50;
+ data[i] = (std::rand() % (max * 2)) - max;
Review comment:
How about change to `data[i] = std::rand() * 1.0 / RAND_MAX - 0.5;`? As `max
= 1` will only generate two values: -1.0and 0.0 .
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] ciyongch commented on a change in pull request #17318: Enable MKL-DNN FullyConnected backward
ciyongch commented on a change in pull request #17318: Enable MKL-DNN
FullyConnected backward
URL: https://github.com/apache/incubator-mxnet/pull/17318#discussion_r379389375
##
File path: tests/cpp/include/test_mkldnn.h
##
@@ -63,24 +63,24 @@ struct TestArrayShapes {
};
// Init arrays with the default layout.
-inline static void InitDefaultArray(NDArray *arr, bool is_rand = false) {
+inline static void InitDefaultArray(NDArray *arr, bool is_rand = false, int
max = 50) {
const TBlob = arr->data();
mshadow::default_real_t *data = blob.dptr();
int size = blob.Size();
for (int i = 0; i < size; i++)
if (is_rand) {
- data[i] = (std::rand() % 100) - 50;
+ data[i] = (std::rand() % (max * 2)) - max;
} else {
- data[i] = i % 100 - 50;
+ data[i] = i % (max * 2) - max;
Review comment:
Same as above, how about change to something like `data[i] = i * 2.0 / size
- 1.0` to generate [-1.0, 1.0)?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
