[GitHub] [incubator-mxnet] Kh4L commented on a change in pull request #18749: Refactor Gluon parameter serialization format

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

Kh4L commented on a change in pull request #18749:
URL: https://github.com/apache/incubator-mxnet/pull/18749#discussion_r456731407



##########
File path: python/mxnet/gluon/block.py
##########
@@ -336,41 +337,43 @@ def _collect_params_with_prefix(self, prefix='', select=None):
             ret.update(child()._collect_params_with_prefix(prefix + name, select))
         return ret
 
-    def save_parameters(self, filename, deduplicate=False):
-        """Save parameters to file.
+    def save_parameters(self, filename):
+        """Save parameters to file based on numpy .npz format.
 
-        Saved parameters can only be loaded with `load_parameters`. Note that this
-        method only saves parameters, not model structure. If you want to save
-        model structures, please use :py:meth:`HybridBlock.export`.
+        Saved parameters can be loaded with `load_parameters` or numpy.load.
+        Note that this method only saves parameters, not model structure. If
+        you want to save model structures, please use :py:meth:`HybridBlock.export`.
 
         Parameters
         ----------
-        filename : str
-            Path to file.
-        deduplicate : bool, default False
-            If True, save shared parameters only once. Otherwise, if a Block
-            contains multiple sub-blocks that share parameters, each of the
-            shared parameters will be separately saved for every sub-block.
+        filename : str or file
+            Either the filename (string) or an open file (file-like object)
+            where the data will be saved.
 
         References
         ----------
         `Saving and Loading Gluon Models \
         <https://mxnet.apache.org/api/python/docs/tutorials/packages/gluon/blocks/save_load_params.html>`_
-        """
-        params = self._collect_params_with_prefix()
-
-        if deduplicate:
-            # Shared parameters are stored only a single time as of MXNet 1.6.
-            # Shared parameters are registered under multiple prefixes returned by
-            # _collect_params_with_prefix. We select a single one and only store
-            # it. In load_parameters it is sufficient for a shared parameter to
-            # only set it for a single prefix.
-            reverse_params = {v: k for k, v in params.items()}
-            params = {v: k for k, v in reverse_params.items()}
 
-        arg_dict = {key: val._reduce() for key, val in params.items()}
-        save_fn = _mx_npx.save if is_np_array() else ndarray.save
-        save_fn(filename, arg_dict)
+        """
+        params_to_names = {}
+        for name, param in self._collect_params_with_prefix().items():
+            params_to_names.setdefault(param, []).append(name)
+        params = {}
+        for param, names in params_to_names.items():
+            assert len(names)
+            params[names[0]] = param._reduce().asnumpy()
+            for name in names[1:]:
+                # Shared parameters are known under multiple names. We save the
+                # parameter according to it's first name and save the mapping

Review comment:
       Typo nit: `its`




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