You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@singa.apache.org by GitBox <gi...@apache.org> on 2020/06/12 22:16:53 UTC
[GitHub] [singa] XJDKC opened a new pull request #731: fix the bug of param name
XJDKC opened a new pull request #731:
URL: https://github.com/apache/singa/pull/731
This pr contains:
1. fix the bug of extra separator before the parameter name
2. add comments for layer and model
3. move the old test cases from test_module to test_model
4. fix bugs in test_model
5. fix bugs of AnalyzeNodes when running graph in serial
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440582387
##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
self.name = ''
return self.name
Review comment:
2. This scheme will reset the layer name many times.
```python
1. model.__init__
2. layer1.__init__
3. layer2.__init__
4. layer3.__init__
5. layer2.__setattr__: reset the name of layer3
6. layer1.__setattr__: reset the names of layer2 and layer3
7. model.__setattr__: reset the names of laye1, layer2 and layer3
```
And we can't get the params before the initialization because they are not created yet at that time.
3. the tensor has a default name if user doesn't set the name.https://github.com/apache/singa/blob/master/python/singa/tensor.py#L116
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440735160
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
That's the current implementation.https://github.com/apache/singa/pull/731/files#diff-3240fd3b573c3eaf8f8d6da8470e68e7R232
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r439708239
##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
self.name = ''
return self.name
Review comment:
1. If _get_unqiue_name is called twice, tha layer name will have duplicateed substrings.
2. I think we can't implement like this, we need to set the name after calling the __init__. If there is a multi-level structure(model->layer1->layer2), the order of function calls is as follows:
* model.\_\_init\_\_
* layer1.\_\_init\_\_
* layer2.\_\_init\_\_
* layer1.\_\_setattr\_\_: the global name of layer1 hasn't been set at this time(just a local name), so we can't set the name for layer2.
* model.\_\_setattr\_\_
3. I have thought about this, but we may set names for non-parameter tensors. If users set a name for a tensor that is not a parameter, this method will overwrite the 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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440582387
##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
self.name = ''
return self.name
Review comment:
2. This scheme will reset the layer name many times.
```python
1. model.__init__
2. layer1.__init__
3. layer2.__init__
4. layer3.__init__
5. layer2.__setattr__: reset the name of layer3
6. layer1.__setattr__: reset the names of layer2 and layer3
7. model.__setattr__: reset the names of laye1, layer2 and layer3
```
3. the tensor has a default name if user doesn't set the name.https://github.com/apache/singa/blob/master/python/singa/tensor.py#L116
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440720540
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
So users name the param tensors, we should raise a warning? Now, we name layers and param tensors automatically in __setattr__.
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440742954
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
Thanks😄
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440735160
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
That's just the current implementation.https://github.com/apache/singa/pull/731/files#diff-3240fd3b573c3eaf8f8d6da8470e68e7R232
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r439708239
##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
self.name = ''
return self.name
Review comment:
1. If _get_unqiue_name is called twice, tha layer name will have duplicateed substrings.
2. I think we can't implement like this, we need to set the name after calling the __init__. If there is a multi-level structure(model->layer1->layer2), the order of function calls is as follows
* model.\_\_init\_\_
* layer1.\_\_init\_\_
* layer2.\_\_init\_\_
* layer1.\_\_setattr\_\_: the global name of layer1 hasn't been set at this time(just a local name), so we can't set the name of layer2.
* model.\_\_setattr\_\_
3. I have thought about this, but we may set names for non-parameter tensors. If users set a name for a tensor that is not a parameter, this method will overwrite the 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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440735160
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
That's the current implementation.https://github.com/apache/singa/pull/731/files#diff-3240fd3b573c3eaf8f8d6da8470e68e7R232
If users name a param, the tensor is not a dummy tensor. So we won't change its 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
[GitHub] [singa] nudles commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
nudles commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440718156
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
I mean if we use ` w_name = self.get_param_name('W')`, then we can also write ` w_name = self.get_param_name('Weights')`.
I think we should not let the users name the param tensors to avoid such issue.
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440732669
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
We can't implement like that. \_\_setattr\_\_ will be called if users want to set an attr. But if they modify the name of the Tensor like this`W.name = "Weight"`, \_\_setattr\_\_ won't be called. And we can't prevent users from changing the name by adding Tensor.\_\_setattr\_\_ because this tensor might not a param of a layer.
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
We can't implement like that. \_\_setattr\_\_ will be called if users want to set an attr. But if they modify the name of the Tensor like this`W.name = "Weight"`, \_\_setattr\_\_ won't be called. And we can't prevent users from changing the name by adding Tensor.\_\_setattr\_\_ because this tensor might not be a param of a layer.
----------------------------------------------------------------
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
[GitHub] [singa] nudles commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
nudles commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440548205
##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
self.name = ''
return self.name
Review comment:
2.
```python
def __set_attr__(self, name, value):
if isinstance(value, layer) and layer.name is None: # set the name of a layer attribute
if self.name != "":
prefix = self.name + Layer.sep
else:
prefix = ""
value.name = prefix + name
elif name == 'name': # reset the name of a layer attribute; triggered by the line above.
for sublayer in self.sublayers:
sublayer.name = value + Layer.sep + sublayer.name.split(Layer.sep)[-1]
for pname, pval in self.get_params():
pval.name = value + Param.sep + pval.name.split(Param.sep)[-1]
```
3. we keep the user assigned name for tensors and do nothing for such cases.
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440720540
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
So users name the param tensors, we should raise a warning?
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440640754
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
Has addressed the problems mentioned above. Ready to merge.
----------------------------------------------------------------
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
[GitHub] [singa] nudles commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
nudles commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440722489
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
if a user set the name of a param explicitly, then we should not change it implicitly by prepending a prefix to it.
e.g., if a user call `W.name = "Weight"`, then we should not change it to "layer1.conv1.Weight"
----------------------------------------------------------------
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
[GitHub] [singa] nudles commented on a change in pull request #731: fix the bug of param name
Posted by 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
[GitHub] [singa] nudles commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
nudles commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440737845
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
I missed your updates...
I will merge it soon.
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r439708239
##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
self.name = ''
return self.name
Review comment:
1. If _get_unqiue_name is called twice, tha layer name will have duplicateed substrings.
2. I think we can't implement like this, we need to set the name after calling the __init__. If there is a multi-level structure(model->layer1->layer2), the order of function calls is as follows
* model.\_\_init\_\_
* layer1.\_\_init\_\_
* layer2.\_\_init\_\_
* layer1.\_\_setattr\_\_: the global name of layer1 hasn't been set at this time(just a local name), so we can't set the name for layer2.
* model.\_\_setattr\_\_
3. I have thought about this, but we may set names for non-parameter tensors. If users set a name for a tensor that is not a parameter, this method will overwrite the 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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440720540
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
So users name the param tensors, we should raise a warning? Now, we name layers and param tensors automatically in \_\_setattr\_\_.
----------------------------------------------------------------
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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r439708239
##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
self.name = ''
return self.name
Review comment:
1. If _get_unqiue_name is called twice, tha layer name will have duplicateed substrings.
2. I think we can't implement like this, we need to set the name after calling the __init__. If there is a multi-level structure(model->layer1->layer2), the order of function calls is as follows
* model.__init__
* layer1.__init__
* layer2.__init__
* layer1.__setattr__: the global name of layer1 hasn't been set at this time(just a local name), so we can't set the name of layer2.
* model.__setattr__
3. I have thought about this, but we may set names for non-parameter tensors. If users set a name for a tensor that is not a parameter, this method will overwrite the 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
[GitHub] [singa] XJDKC commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
XJDKC commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440582387
##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
self.name = ''
return self.name
Review comment:
2. This scheme will reset the layer name many times.
```python
1. model.\_\_init\_\_
2. layer1.\_\_init\_\_
3. layer2.\_\_init\_\_
4. layer3.\_\_init\_\_
5. layer2.\_\_setattr\_\_: reset the name of layer3
6. layer1.\_\_setattr\_\_: reset the names of layer2 and layer3
7. model.\_\_setattr\_\_: reset the names of laye1, layer2 and layer3
```
3. the tensor has a default name if user doesn't set the name.https://github.com/apache/singa/blob/master/python/singa/tensor.py#L116
----------------------------------------------------------------
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
[GitHub] [singa] nudles merged pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
nudles merged pull request #731:
URL: https://github.com/apache/singa/pull/731
----------------------------------------------------------------
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
[GitHub] [singa] nudles commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
nudles commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440589149
##########
File path: python/singa/layer.py
##########
@@ -499,7 +581,7 @@ def initialize(self, x):
self.kernel_size[0],
self.kernel_size[1],
)
- w_name = self.name + Layer.sep + 'W'
+ w_name = self.get_param_name('W')
Review comment:
we should avoid this. it means we may a parameter self.W whose name is set to "Weights". It is a bit confusing on the naming.
##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
self.name = ''
return self.name
Review comment:
2. yes. it will set and then reset the names many times if the depths is large. the advantage is that whenever the parent layer name is changed,all sub layers will be updated. I am fine with both schemes. just need to add some docstring to state that the layer name should not change/reset at runtime.
3. then just check based on the pattern? if it is like dummy+integer, then it means the name is not set by users.
----------------------------------------------------------------
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
[GitHub] [singa] nudles commented on a change in pull request #731: fix the bug of param name
Posted by GitBox <gi...@apache.org>.
nudles commented on a change in pull request #731:
URL: https://github.com/apache/singa/pull/731#discussion_r440548205
##########
File path: python/singa/layer.py
##########
@@ -158,6 +217,21 @@ def _get_unique_name(self):
self.name = ''
return self.name
Review comment:
2. we can try to reset the names recursively when layer1's name is set?
```python
def __set_attr__(self, name, value):
if isinstance(value, layer) and layer.name is None: # set the name of a layer attribute
if self.name != "":
prefix = self.name + Layer.sep
else:
prefix = ""
value.name = prefix + name
elif name == 'name': # reset the name of a layer attribute; triggered by the line above.
for sublayer in self.sublayers:
sublayer.name = value + Layer.sep + sublayer.name.split(Layer.sep)[-1]
for pname, pval in self.get_params():
pval.name = value + Param.sep + pval.name.split(Param.sep)[-1]
```
3. we keep the user assigned name for tensors and do nothing for such cases.
----------------------------------------------------------------
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