[GitHub] [singa] XJDKC commented on a change in pull request #694: SINGA-510 Add Time Profiling for Single GPU

[GitBox <gi...@apache.org>]

XJDKC commented on a change in pull request #694:
URL: https://github.com/apache/singa/pull/694#discussion_r427128930



##########
File path: include/singa/core/scheduler.h
##########
@@ -68,6 +76,11 @@ class Node {
   OpFunc op_;
   EdgeVec in_edges_;
   EdgeVec out_edges_;
+
+  string op_name_;
+  float time_elapsed_ = 0;
+  int iteration_ = 0;

Review comment:
       I think we can move this variable to Graph. Although some ops might be executed more times than others when using old APIs.

##########
File path: include/singa/core/device.h
##########
@@ -134,6 +150,8 @@ class Device {
   int num_executors_ = 0;
   unsigned seed_ = 0;
   bool graph_enabled_ = false;
+  string profiling_mode_ = "CHRONO";
+  int verbosity_ = 0;

Review comment:
       How about using two enum classes to distinguish the different profiling mode and the verbosity level?

##########
File path: src/core/scheduler/scheduler.cc
##########
@@ -228,6 +229,60 @@ void Graph::Debug() {
   printf("%s", ss.str().c_str());
 }
 
+void Graph::PrintTimeProfiling() {
+  std::stringstream ss;
+
+  // verbosity level: 1 -> forward and backward propagation time
+  if (device_->verbosity() == 1) {
+    bool forward = true;
+    float forward_time = 0;
+    float backward_time = 0;
+    float time_elapsed;
+
+    for (size_t i = 0; i < nodes_.size(); ++i)
+      if (nodes_[i]->time_elapsed() > 0) {
+        if (forward == true)
+          // check the op of cross entropy backward, after that are backward ops
+          if (nodes_[i]->op_name().find("Backward") != std::string::npos)

Review comment:
       Do all backward propagation meet this condition?

##########
File path: src/model/layer/cudnn_activation.cc
##########
@@ -81,7 +81,7 @@ const Tensor CudnnActivation::Forward(int flag, const Tensor& input) {
     CUDNN_CHECK(cudnnActivationForward(
         ctx->cudnn_handle, this->acti_desc_, &alpha, this->desc_,
         inblock->data(), &beta, this->desc_, outblock->mutable_data()));
-  }, {input.block()}, {output.block()});
+  }, {input.block()}, {output.block()}, "cudnnActivationForward");

Review comment:
       I haven't modified operations in files under the layer dir before, so they may be not well buffered.




----------------------------------------------------------------
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:
users@infra.apache.org