zackchase commented on a change in pull request #7356: decouple record/train
and add state readers
URL: https://github.com/apache/incubator-mxnet/pull/7356#discussion_r131773045
##########
File path: python/mxnet/autograd.py
##########
@@ -112,6 +137,34 @@ def pause(is_train=False):
return RecordingStateScope(False, is_train)
+def override_train():
Review comment:
Thanks @szha for working hard to resolve these issues.
I really like that you've added setters / getters for training and recording
and am relieved to see them decoupled.
So one question is -
Is predict =True equivalent to training=False?
If so then we would only need to set training and I'd agree with Eric that
we could hold off on adding predict() right now for fear of introducing
redundancy. Still we have a tough situation for a modeler who wants at will to
be able to run forward passes with dropout on vs off. It's hard to see how they
can turn dropout on/off without messing with batchnorm.
For me, the main idea for having a predict(): scope would be *if the default
were training=True*. I tend to think that tis should be the case.
**Argument: If you just instantiate a Dropout layer and call forward on it,
you'd expect the Dropout to be applied**
At the same time I'm cognizant of Eric's belief that this breaks from the
previous way of doing things and might harm backwards compatibility in some
sense.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
