[GitHub] [singa] nudles commented on a change in pull request #731: fix the bug of param name

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

nudles commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r439698164



##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
             self.name = ''
         return self.name

Review comment:
       if _get_unique_name is called twice, will the returned name have duplicated substrings?

##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
             self.name = ''
         return self.name

Review comment:
       would it better to create the name in the parent layer?
   ```python
   self.conv = Conv2d()... # when this called, we set self.conv.name = self.name + Layer.sep + 'conv' in _set_attr_
   
   def __set_attr__(self, name, value):
        if isinstance(value, layer):
           ....
          if self.name != "":
            prefix = self.name + Layer.sep
          else:
             prefix = ""
      
           value.name = prefix +  name
        ...
   ```
   

##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
             self.name = ''
         return self.name

Review comment:
       similarly, we can set the name of the params as:
   ```python
   self.W= Tensor()... # when this called, we set self.W.name = self.name + Layer.ParamSep + 'W' in _set_attr_
   
   def __set_attr__(self, name, value):
        if isinstance(value, Tensor):
           ....
          if self.name != "":
            prefix = self.name + Layer.ParamSep
          else:
             prefix = ""
      
           value.name = prefix +  name
        ...
   
   ```




----------------------------------------------------------------
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