support regex of collect_params() #9348

tornadomeet · 2018-01-08T16:26:21Z

Description

#9323

Checklist

Essentials

Passed code style checking (make lint)
Changes are complete (i.e. I finished coding on this PR)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

@szha select name and function comments may be improved. if these change is ok, i'll add unit test here.

piiswrong · 2018-01-08T19:18:22Z

python/mxnet/gluon/block.py

@@ -227,13 +228,37 @@ def params(self):
        children's parameters)."""
        return self._params

-    def collect_params(self):
+    def collect_params(self, select=['.*']):


select should be None by default.

ok, will change this.

piiswrong · 2018-01-08T19:19:03Z

python/mxnet/gluon/block.py

+        Parameters
+        ----------
+        select : list of str
+            List of name or regular expressions


Only support 1 regex, not a list

Done. @piiswrong

another, if user has a python list to select parameter, like ['a', 'b', 'c'], t hen he/she should first convert select='a|b|c' first using some code, any better way to avoid this?

piiswrong · 2018-01-09T19:56:54Z

python/mxnet/gluon/block.py

        ret = ParameterDict(self._params.prefix)
-        ret.update(self.params)
+        if select is None:


don't do any regex matching if select is None. Its very slow

@piiswrong Done.

chinakook · 2018-01-10T11:44:42Z

wonderful.

szha · 2018-01-10T19:38:16Z

python/mxnet/gluon/block.py

+        else:
+            for name in self.params.keys():
+                if re.compile(select).match(name):
+                    ret.update({name:self.params[name]})


pattern = re.compile(select) ret.update({name: value for name, value in self.params.items() if pattern.match(name)}

Compiling a pattern that don't change can be expensive especially inside the for loop. I wonder if we should cache the compiled object and pass it to the recursive calls.

@szha Done.

tornadomeet · 2018-01-12T01:19:08Z

python/mxnet/gluon/block.py

@@ -253,7 +253,7 @@ def collect_params(self, select=None):
        The selected :py:class:`ParameterDict`
        """
        ret = ParameterDict(self._params.prefix)
-        if select is None:
+        if not select:


if select = [] or {} or '' or () then not select will be True, may be wrong here.

You're right, and it should be fine as long as both empty string and None returns true.

…lter

* support regex of collect_params() * fix pylint * change default value && make select as a sigle reg * fix if select is None, then will not do any regex matching * update regex compile && add test * Update block.py * support regex of collect_params() * fix pylint * change default value && make select as a sigle reg * fix if select is None, then will not do any regex matching * update regex compile && add test

piiswrong reviewed Jan 8, 2018

View reviewed changes

piiswrong reviewed Jan 9, 2018

View reviewed changes

szha reviewed Jan 10, 2018

View reviewed changes

wuwei and others added 6 commits January 11, 2018 16:15

support regex of collect_params()

d3c9ee4

fix pylint

6294744

change default value && make select as a sigle reg

0f4fe1e

fix if select is None, then will not do any regex matching

7d15253

update regex compile && add test

ac25025

Update block.py

d565e6d

tornadomeet commented Jan 12, 2018

View reviewed changes

wuwei and others added 6 commits January 12, 2018 10:00

support regex of collect_params()

1bcde15

fix pylint

dd28916

change default value && make select as a sigle reg

b4c9b0c

fix if select is None, then will not do any regex matching

5712b52

update regex compile && add test

818ce58

Merge branch 'filter' of https://github.com/tornadomeet/mxnet into fi…

0cee605

…lter

piiswrong merged commit 4600070 into apache:master Jan 12, 2018

tornadomeet deleted the filter branch January 14, 2018 07:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support regex of collect_params() #9348

support regex of collect_params() #9348

tornadomeet commented Jan 8, 2018 •

edited

Loading

piiswrong Jan 8, 2018

tornadomeet Jan 9, 2018

piiswrong Jan 8, 2018

tornadomeet Jan 9, 2018 •

edited

Loading

piiswrong Jan 9, 2018

tornadomeet Jan 10, 2018

chinakook commented Jan 10, 2018

szha Jan 10, 2018 •

edited

Loading

szha Jan 10, 2018

tornadomeet Jan 11, 2018

tornadomeet Jan 12, 2018

szha Jan 12, 2018

support regex of collect_params() #9348

support regex of collect_params() #9348

Conversation

tornadomeet commented Jan 8, 2018 • edited Loading

Description

Checklist

Essentials

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tornadomeet Jan 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinakook commented Jan 10, 2018

szha Jan 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tornadomeet commented Jan 8, 2018 •

edited

Loading

tornadomeet Jan 9, 2018 •

edited

Loading

szha Jan 10, 2018 •

edited

Loading