[GitHub] [incubator-mxnet] ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate array manipulation related operators
ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate array manipulation related operators URL: https://github.com/apache/incubator-mxnet/pull/17487#discussion_r385342488 ## File path: benchmark/opperf/README.md ## @@ -72,6 +73,8 @@ python incubator-mxnet/benchmark/opperf/opperf.py --output-format json --output- 3. **dtype** : By default, `float32`. You can override and set the global dtype for all operator benchmarks. Example: --dtype float64. +4. **profiler** : By default, 'native'. You can override and set the global profiler for all operator benchmarks. Example: --profiler 'python'. Review comment: Added a line. @apeforest 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] ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate array manipulation related operators
ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate
array manipulation related operators
URL: https://github.com/apache/incubator-mxnet/pull/17487#discussion_r379225052
##
File path: benchmark/opperf/utils/op_registry_utils.py
##
@@ -137,26 +140,24 @@ def prepare_op_inputs(op, arg_params):
arg_values[arg_name] = DEFAULTS_INPUTS["dtype_int"]
elif (op.startswith(('random','sample')) or op in float_only) and
arg_name == "dtype":
arg_values[arg_name] = DEFAULTS_INPUTS["dtype_float"]
-elif "NDArray" in arg_type and op == "ravel_multi_index":
-arg_values[arg_name] = DEFAULTS_INPUTS["ravel_data"]
elif op in custom_data and arg_name + "_" + op.lower() in
DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_" + op.lower()]
-elif "NDArray" in arg_type and arg_name + "_nd" in DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_nd"]
-elif "NDArray" in arg_type and op in ops_4d and arg_name + "_4d" in
DEFAULTS_INPUTS:
+elif op in ops_4d and arg_name + "_4d" in DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_4d"]
-elif "NDArray" in arg_type and op in ops_3d and arg_name + "_3d" in
DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_3d"]
-elif "NDArray" in arg_type and op == 'softmax_cross_entropy':
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_smce"]
+elif op in ops_dim1 and arg_name + "_dim1" in DEFAULTS_INPUTS:
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_dim1"]
+elif "NDArray" in arg_type:
+if op == "ravel_multi_index":
+arg_values[arg_name] = DEFAULTS_INPUTS["ravel_data"]
+elif arg_name + "_nd" in DEFAULTS_INPUTS:
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_nd"]
+elif op in ops_3d and arg_name + "_3d" in DEFAULTS_INPUTS:
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_3d"]
+elif op == 'softmax_cross_entropy':
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_smce"]
+# default case
elif arg_name in DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name]
-elif "float" in arg_type and arg_name + "_float" in DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_float"]
-elif "Shape" in arg_type and arg_name + "_shape" in DEFAULTS_INPUTS:
Review comment:
Another non reachable if condition.
axis and axis_shape both exist. And since `if arg_name in DEFAULTS_INPUTS:`
is checked before ` arg_name + "_shape" in DEFAULTS_INPUTS` it will never be
reached.
Hence removed it.
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] ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate array manipulation related operators
ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate
array manipulation related operators
URL: https://github.com/apache/incubator-mxnet/pull/17487#discussion_r385322042
##
File path: benchmark/opperf/utils/op_registry_utils.py
##
@@ -137,26 +140,24 @@ def prepare_op_inputs(op, arg_params):
arg_values[arg_name] = DEFAULTS_INPUTS["dtype_int"]
elif (op.startswith(('random','sample')) or op in float_only) and
arg_name == "dtype":
arg_values[arg_name] = DEFAULTS_INPUTS["dtype_float"]
-elif "NDArray" in arg_type and op == "ravel_multi_index":
-arg_values[arg_name] = DEFAULTS_INPUTS["ravel_data"]
elif op in custom_data and arg_name + "_" + op.lower() in
DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_" + op.lower()]
-elif "NDArray" in arg_type and arg_name + "_nd" in DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_nd"]
-elif "NDArray" in arg_type and op in ops_4d and arg_name + "_4d" in
DEFAULTS_INPUTS:
+elif op in ops_4d and arg_name + "_4d" in DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_4d"]
-elif "NDArray" in arg_type and op in ops_3d and arg_name + "_3d" in
DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_3d"]
-elif "NDArray" in arg_type and op == 'softmax_cross_entropy':
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_smce"]
+elif op in ops_dim1 and arg_name + "_dim1" in DEFAULTS_INPUTS:
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_dim1"]
+elif "NDArray" in arg_type:
Review comment:
incorrect logic hence reverted
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] ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate array manipulation related operators
ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate
array manipulation related operators
URL: https://github.com/apache/incubator-mxnet/pull/17487#discussion_r379223963
##
File path: benchmark/opperf/utils/op_registry_utils.py
##
@@ -137,26 +140,24 @@ def prepare_op_inputs(op, arg_params):
arg_values[arg_name] = DEFAULTS_INPUTS["dtype_int"]
elif (op.startswith(('random','sample')) or op in float_only) and
arg_name == "dtype":
arg_values[arg_name] = DEFAULTS_INPUTS["dtype_float"]
-elif "NDArray" in arg_type and op == "ravel_multi_index":
-arg_values[arg_name] = DEFAULTS_INPUTS["ravel_data"]
elif op in custom_data and arg_name + "_" + op.lower() in
DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_" + op.lower()]
-elif "NDArray" in arg_type and arg_name + "_nd" in DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_nd"]
-elif "NDArray" in arg_type and op in ops_4d and arg_name + "_4d" in
DEFAULTS_INPUTS:
+elif op in ops_4d and arg_name + "_4d" in DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_4d"]
-elif "NDArray" in arg_type and op in ops_3d and arg_name + "_3d" in
DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_3d"]
-elif "NDArray" in arg_type and op == 'softmax_cross_entropy':
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_smce"]
+elif op in ops_dim1 and arg_name + "_dim1" in DEFAULTS_INPUTS:
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_dim1"]
+elif "NDArray" in arg_type:
Review comment:
clubbed all the conditions falling specifically for NDArray args under the
hood of NDArray for better readability.
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] ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate array manipulation related operators
ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate
array manipulation related operators
URL: https://github.com/apache/incubator-mxnet/pull/17487#discussion_r379225052
##
File path: benchmark/opperf/utils/op_registry_utils.py
##
@@ -137,26 +140,24 @@ def prepare_op_inputs(op, arg_params):
arg_values[arg_name] = DEFAULTS_INPUTS["dtype_int"]
elif (op.startswith(('random','sample')) or op in float_only) and
arg_name == "dtype":
arg_values[arg_name] = DEFAULTS_INPUTS["dtype_float"]
-elif "NDArray" in arg_type and op == "ravel_multi_index":
-arg_values[arg_name] = DEFAULTS_INPUTS["ravel_data"]
elif op in custom_data and arg_name + "_" + op.lower() in
DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_" + op.lower()]
-elif "NDArray" in arg_type and arg_name + "_nd" in DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_nd"]
-elif "NDArray" in arg_type and op in ops_4d and arg_name + "_4d" in
DEFAULTS_INPUTS:
+elif op in ops_4d and arg_name + "_4d" in DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_4d"]
-elif "NDArray" in arg_type and op in ops_3d and arg_name + "_3d" in
DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_3d"]
-elif "NDArray" in arg_type and op == 'softmax_cross_entropy':
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_smce"]
+elif op in ops_dim1 and arg_name + "_dim1" in DEFAULTS_INPUTS:
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_dim1"]
+elif "NDArray" in arg_type:
+if op == "ravel_multi_index":
+arg_values[arg_name] = DEFAULTS_INPUTS["ravel_data"]
+elif arg_name + "_nd" in DEFAULTS_INPUTS:
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_nd"]
+elif op in ops_3d and arg_name + "_3d" in DEFAULTS_INPUTS:
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_3d"]
+elif op == 'softmax_cross_entropy':
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_smce"]
+# default case
elif arg_name in DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name]
-elif "float" in arg_type and arg_name + "_float" in DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_float"]
-elif "Shape" in arg_type and arg_name + "_shape" in DEFAULTS_INPUTS:
Review comment:
Another non reachable if condition.
axis and axis_shape both exist. And since `if arg_name in DEFAULTS_INPUTS:`
is checked before ` arg_name + "_shape" in DEFAULTS_INPUTS` it will never be
reached.
Hence moved up before the default case check.
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] ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate array manipulation related operators
ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate array manipulation related operators URL: https://github.com/apache/incubator-mxnet/pull/17487#discussion_r379223794 ## File path: benchmark/opperf/rules/default_params.py ## @@ -238,7 +247,13 @@ "data_3d": DEFAULT_DATA_3d, "label_smce": DEFAULT_LABEL_SMCE, "label": DEFAULT_LABEL, - "index": DEFAULT_INDEX, Review comment: this was duplicate hence removed it. 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] ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate array manipulation related operators
ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate
array manipulation related operators
URL: https://github.com/apache/incubator-mxnet/pull/17487#discussion_r379224557
##
File path: benchmark/opperf/utils/op_registry_utils.py
##
@@ -137,26 +140,24 @@ def prepare_op_inputs(op, arg_params):
arg_values[arg_name] = DEFAULTS_INPUTS["dtype_int"]
elif (op.startswith(('random','sample')) or op in float_only) and
arg_name == "dtype":
arg_values[arg_name] = DEFAULTS_INPUTS["dtype_float"]
-elif "NDArray" in arg_type and op == "ravel_multi_index":
-arg_values[arg_name] = DEFAULTS_INPUTS["ravel_data"]
elif op in custom_data and arg_name + "_" + op.lower() in
DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_" + op.lower()]
-elif "NDArray" in arg_type and arg_name + "_nd" in DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_nd"]
-elif "NDArray" in arg_type and op in ops_4d and arg_name + "_4d" in
DEFAULTS_INPUTS:
+elif op in ops_4d and arg_name + "_4d" in DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_4d"]
-elif "NDArray" in arg_type and op in ops_3d and arg_name + "_3d" in
DEFAULTS_INPUTS:
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_3d"]
-elif "NDArray" in arg_type and op == 'softmax_cross_entropy':
-arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_smce"]
+elif op in ops_dim1 and arg_name + "_dim1" in DEFAULTS_INPUTS:
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_dim1"]
+elif "NDArray" in arg_type:
+if op == "ravel_multi_index":
+arg_values[arg_name] = DEFAULTS_INPUTS["ravel_data"]
+elif arg_name + "_nd" in DEFAULTS_INPUTS:
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_nd"]
+elif op in ops_3d and arg_name + "_3d" in DEFAULTS_INPUTS:
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_3d"]
+elif op == 'softmax_cross_entropy':
+arg_values[arg_name] = DEFAULTS_INPUTS[arg_name + "_smce"]
+# default case
elif arg_name in DEFAULTS_INPUTS:
arg_values[arg_name] = DEFAULTS_INPUTS[arg_name]
-elif "float" in arg_type and arg_name + "_float" in DEFAULTS_INPUTS:
Review comment:
both ifs are never reached because the default case "arg_name" in
DEFAULTS_INPUTS is reached already. It is the base case.
for eg if there are 2 keys "x" and "x_float" it will always find x thanks to
`elif arg_name in DEFAULTS_INPUTS:`
it will never reach `arg_name + "_float" in DEFAULTS_INPUTS`
Hence had to be moved up.
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] ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate array manipulation related operators
ChaiBapchya commented on a change in pull request #17487: [OpPerf] Consolidate array manipulation related operators URL: https://github.com/apache/incubator-mxnet/pull/17487#discussion_r375443671 ## File path: benchmark/opperf/README.md ## @@ -72,6 +73,8 @@ python incubator-mxnet/benchmark/opperf/opperf.py --output-format json --output- 3. **dtype** : By default, `float32`. You can override and set the global dtype for all operator benchmarks. Example: --dtype float64. +4. **profiler** : By default, 'native'. You can override and set the global profiler for all operator benchmarks. Example: --profiler 'python'. Review comment: Native profiler uses the MXNet's default profiler function Python uses python's time-it function 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
