[Wait for #2710][Layer] add "subtract layer" @open sesame 10/11 15:45

[baek2sm]

• added "subtract layer"

Self evaluation:

1. Build test: [X]Passed [ ]Failed [ ]Skipped
2. Run test: [X]Passed [ ]Failed [ ]Skipped

Signed-off-by: Seungbaek Hong [email protected]

[taos-ci]

:memo: TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2724. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/.

[baek2sm]

The add and sub layers have almost identical code. How about creating a basic operation class? It would be simple if the add and sub inherited from the basic operation class. Multiplication and division also seem to have the same structure.

That sounds good. Let's think together to find an improved structure.

[baek2sm]

Nice work! One minor suggestion is to rename it SubtractLayer (SubLayer sounds like a subunit of a layer). What do you think?

That's a good idea. I'll rename it! (and the other, too)

[jijoongmoon]

The add and sub layers have almost identical code. How about creating a basic operation class? It would be simple if the add and sub inherited from the basic operation class. Multiplication and division also seem to have the same structure.

I also think same way. How about unary and binary layer.

[SeoHyungjun]

The add and sub layers have almost identical code. How about creating a basic operation class? It would be simple if the add and sub inherited from the basic operation class. Multiplication and division also seem to have the same structure.

That sounds good. Let's think together to find an improved structure.

How about creating the operation layer like this?

// operation_layer.h

#pragma once    

#include <tensor.h>    

namespace nntrainer {    

template <typename OP>    
class OperationLayer {    
public:    
  virtual ~OperationLayer() = default;    

protected:    
  OperationLayer() = default;    

  virtual void finalize(InitLayerContext &context) = 0;    

  virtual void forwarding(RunLayerContext &context, bool training) = 0;    

  virtual void incremental_forwarding(RunLayerContext &context,    
                                      unsigned int from, unsigned int to,    
                                      bool training) = 0;    

  virtual void calcDerivative(RunLayerContext &context) = 0;    

  virtual void setProperty(const std::vector<std::string> &values) = 0;    

  
void commonForwarding(RunLayerContext &context, bool training,    
                       const Tensor &input0, const Tensor &input1,    
                       Tensor &hidden_);    

// Implementing functions for common codes
};    

template <>    
struct OperationLayer<ADDITION> {    
  static constexpr const char *type = "add";    
};    

template <>    
struct OperationLayer<SUBTRACTION> {    
  static constexpr const char *type = "sub";    
};    

template <>    
struct OperationLayer<MULTIPLICATION> {    
  static constexpr const char *type = "mul";    
};    

template <>    
struct OperationLayer<DIVISION> {    
  static constexpr const char *type = "div";    
};    

} // namespace nntrainer    

// operation_layer.cpp

template <typename OP>  
void OperationLayer<OP>::commonForwarding(RunLayerContext& context, bool training,  
                                          const Tensor& input0, const Tensor& input1, Tensor& hidden_) {  
  switch (OP::type) {  
    case "add":  
      input0.add(input1, hidden_);  
      break;  
    case "sub":  
      input0.subtract(input1, hidden_);  
      break;  
    case "mul":  
      input0.multiply(input1, hidden_);  
      break;  
    case "div": 
      input0.divide(input1, hidden_);  
      break;  
    default:  
      throw std::runtime_error("Invalid operator type");  
  }  
}  

// implementation of other common function

[SeoHyungjun]

I think we can create an add layer like this using the operation layer.

// add_layer.h
#pragma once  

#include "operation_layer.h"  

namespace nntrainer {  

class AddLayer : public OperationLayer<ADDITION> {  
public:  
  using OperationLayer<ADDITION>::OperationLayer;  

  void finalize(InitLayerContext &context) override;  

  void forwarding(RunLayerContext &context, bool training) override;  

  void incremental_forwarding(RunLayerContext &context, unsigned int from,  
                              unsigned int to, bool training) override;  

  void calcDerivative(RunLayerContext &context) override;  

  void setProperty(const std::vector<std::string> &values) override;  
};  

} // namespace nntrainer  

//add_layer.cpp

#include "add_layer.h"  

namespace nntrainer {  

void AddLayer::finalize(InitLayerContext &context) {  
  // Add Layer specific initialization code goes here...  
}  

void AddLayer::forwarding(RunLayerContext &context, bool training) {  
  commonForwarding(context, training, input0, input1, hidden_);  
}  

void AddLayer::incremental_forwarding(RunLayerContext &context, unsigned int from,  
                                      unsigned int to, bool training) {  
  // Add Layer specific incremental forward propagation code goes here...  
}  

void AddLayer::calcDerivative(RunLayerContext &context) {  
  // Add Layer specific backpropagation code goes here...  
}  

void AddLayer::setProperty(const std::vector<std::string> &values) {  
  // Add Layer specific property setting code goes here...  
}  

} // namespace nntrainer 

There is a lot of overlap in the currently implemented code. It seems that all other functions can be expressed simply, just like using commonForwarding in the forwarding function.

If each function requires completely different implementation codes, it may be possible to implement its own code rather than a common function in each class.

[EunjuYang]

Great suggestion @SeoHyungjun ! Upon his suggestion, what about using enum for operation type? If more operations are added, the switch clause may not work only with the first character. Also it can improve readability :)

[SeoHyungjun]

Great suggestion @SeoHyungjun ! Upon his suggestion, what about using enum for operation type? If more operations are added, the switch clause may not work only with the first character. Also it can improve readability :)

Good point, @EunjuYang!! As you mentioned, using enums would be beneficial for readability and maintenance purposes!

I have written down the code with added enums below.

// operation_layer.h  

#pragma once      

#include <tensor.h>      

namespace nntrainer {      

enum class OpType {  
  ADDITION,  
  SUBTRACTION,  
  MULTIPLICATION,  
  DIVISION  
};  

template <OpType op_type>      
class OperationLayer {      
public:      
  virtual ~OperationLayer() = default;      

protected:      
  OperationLayer() = default;      

  virtual void finalize(InitLayerContext &context) = 0;      

  virtual void forwarding(RunLayerContext &context, bool training) = 0;      

  virtual void incremental_forwarding(RunLayerContext &context,      
                                      unsigned int from, unsigned int to,      
                                      bool training) = 0;      

  virtual void calcDerivative(RunLayerContext &context) = 0;      

  virtual void setProperty(const std::vector<std::string> &values) = 0;      

    
void commonForwarding(RunLayerContext &context, bool training,      
                       const Tensor &input0, const Tensor &input1,      
                       Tensor &hidden_);      

// Implementing functions for common codes  
};      

template <>      
struct OperationLayer<OpType::ADDITION> {      
  static constexpr const char *type = "add";      
};      

template <>      
struct OperationLayer<OpType::SUBTRACTION> {      
  static constexpr const char *type = "sub";      
};      

template <>      
struct OperationLayer<OpType::MULTIPLICATION> {      
  static constexpr const char *type = "mul";      
};      

template <>      
struct OperationLayer<OpType::DIVISION> {      
  static constexpr const char *type = "div";      
};      

} // namespace nntrainer      

// operation_layer.cpp  

template <OpType op_type>    
void OperationLayer<op_type>::commonForwarding(RunLayerContext& context, bool training,    
                                              const Tensor& input0, const Tensor& input1, Tensor& hidden_) {    
  switch (op_type) {    
    case OpType::ADDITION:    
      input0.add(input1, hidden_);    
      break;    
    case OpType::SUBTRACTION:    
      input0.subtract(input1, hidden_);    
      break;    
    case OpType::MULTIPLICATION:    
      input0.multiply(input1, hidden_);    
      break;    
    case OpType::DIVISION:   
      input0.divide(input1, hidden_);    
      break;    
    default:    
      throw std::runtime_error("Invalid operator type");    
  }    
}    

// implementation of other common function  

// add_layer.h  

#pragma once    

#include "operation_layer.h"    

namespace nntrainer {    

class AddLayer : public OperationLayer<OpType::ADDITION> {    
public:    
  using OperationLayer<OpType::ADDITION>::OperationLayer;    

  void finalize(InitLayerContext &context) override;    

  void forwarding(RunLayerContext &context, bool training) override;    

  void incremental_forwarding(RunLayerContext &context, unsigned int from,    
                              unsigned int to, bool training) override;    

  void calcDerivative(RunLayerContext &context) override;    

  void setProperty(const std::vector<std::string> &values) override;    
};    

} // namespace nntrainer     

//add_layer.cpp  

#include "add_layer.h"    

namespace nntrainer {    

void AddLayer::finalize(InitLayerContext &context) {    
  // Add Layer specific initialization code goes here...    
}    

void AddLayer::forwarding(RunLayerContext &context, bool training) {    
  commonForwarding(context, training, input0, input1, hidden_);    
}    

void AddLayer::incremental_forwarding(RunLayerContext &context, unsigned int from,    
                                      unsigned int to, bool training) {    
  // Add Layer specific incremental forward propagation code goes here...    
}    

void AddLayer::calcDerivative(RunLayerContext &context) {    
  // Add Layer specific backpropagation code goes here...    
}    

void AddLayer::setProperty(const std::vector<std::string> &values) {    
  // Add Layer specific property setting code goes here...    
}    

} // namespace nntrainer   

[baek2sm]

@SeoHyungjun It's a good suggestion. I'll try to reflect on it. Thank you!

[baek2sm]

I've completed the revisions after incorporating feedbacks. Thanks a lot!

[taos-ci]

:octocat: cibot: @baek2sm, A builder checker could not be completed because one of the checkers is not completed. In order to find out a reason, please go to http://ci.nnstreamer.ai/nntrainer/ci/repo-workers/pr-checker/2724-202411061131060.035284996032715-13091f071b159bb56d9d5e3b71eefb7a4805066e/.

[taos-ci]

:octocat: cibot: @baek2sm, A builder checker could not be completed because one of the checkers is not completed. In order to find out a reason, please go to http://ci.nnstreamer.ai/nntrainer/ci/repo-workers/pr-checker/2724-202411061645350.87372088432312-f3aad560a9d01bba11c3cfc53c0dee8b2ad54f7d/.