Layer factory #21
Layer factory #21
Conversation
xfer/model_handler/model_handler.py
Outdated
| prev_symbols=prev_symbols, id2name=id2name, data_name=self.data_name) | ||
| net_symbol_dict = self._get_symbol_dict(self.symbol) | ||
|
|
||
| for layer_symbol in reversed(layer_list): |
There was a problem hiding this comment.
A comment explaining why list is read in reversed order would be helpful
xfer/model_handler/model_handler.py
Outdated
|
|
||
| net_symbol_dict = self._get_symbol_dict(self.symbol) | ||
| for layer_symbol in layer_list: | ||
| added_layer_names.append(layer_symbol.name) |
There was a problem hiding this comment.
Consider incrementing this at the end of the loop after net_symbol_dict update is complete
|
Closing this PR for now as we discussed that the changes are not robust yet. |
Co-Authored-By: jnkm <jmas@live.co.uk>
Co-Authored-By: jnkm <jmas@live.co.uk>
Co-Authored-By: jnkm <jmas@live.co.uk>
Co-Authored-By: jnkm <jmas@live.co.uk>
Co-Authored-By: jnkm <jmas@live.co.uk>
Co-Authored-By: jnkm <jmas@live.co.uk>
xfer/model_handler/model_handler.py
Outdated
| nodes_using_zero_ids_names = [v[consts.NAME] for c, v in enumerate(symbol_dict[consts.NODES]) | ||
| output_layer_names = self._get_output_layer_names(network) | ||
| nodes_using_zero_ids = self._get_layer_ids_with_node_zero_as_input(network[consts.NODES]) | ||
| nodes_using_zero_ids_names = [v[consts.NAME] for c, v in enumerate(network[consts.NODES]) |
There was a problem hiding this comment.
network[consts.NODES] is iterated twice - first to get ids and then to lookup names from ids. Second iteration can be avoided and names can be obtained from the first pass itself.
xfer/model_handler/model_handler.py
Outdated
|
|
||
| self._assert_layer_drop_not_ambiguous(layer_names=nodes_using_zero_ids_names, layer_drop_number=n) | ||
| # The dropped layer is simply the first layer | ||
| drop_layer_name = self._get_name_of_first_operation(network[consts.NODES]) |
There was a problem hiding this comment.
Is this method call necessary? Can't we just take nodes_using_zero_ids_names[0] as the drop_layer_name? If not, add validation check that first operation layer and the layer with node 0 as input are the same.
xfer/model_handler/model_handler.py
Outdated
| if join_deleted: | ||
| layers_dropped.append(join_layer_name) | ||
|
|
||
| for nodes_to_delete, node in enumerate(network[consts.NODES]): |
There was a problem hiding this comment.
This also seems unnecessary. Avoid disconnect between finding the layer with node0 as input and finding the first operation layer. Use a single source of information about which layer to drop and then add validation checks if needed.
Issue #, if available:
Description of changes:
Replaced Layer Factory with use of mxnet.symbol.load_json()
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.