[MXNET-1333] Estimator and Fit API

[roywei]

Description

This PR introduce an Estimator class in contrib with easy fit method to help beginners with model training.
It's been developed on a branch, and we hope to merge it to contrib and get feedback for first iteration.

Design: https://cwiki.apache.org/confluence/display/MXNET/Gluon+Fit+API+-+Tech+Design
JIRA epics: https://issues.apache.org/jira/browse/MXNET-1333
Dev list discussion: https://lists.apache.org/thread.html/13e3dee0fc9dd8e45b6616f97d282096a1ee67cde78a93dada295577@%3Cdev.mxnet.apache.org%3E
Feedbacks: currently all feedbacks are captured in cwiki comment section. We have created JIRA issues for each feedback and will continue to work on it
Follow up PRs:
We currently have the following PRs to address feedback, will create more and track using JIRA issue

1. [MXNET-1358][Fit API] Fit api tutorial
2. [MXNET-1340, 1339][Fit API]Multi input/output
   

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

• The PR title starts with MXNET-1333
• Changes are complete (i.e. I finished coding on this PR)
• 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)
• 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
• Check the API doc at http:https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
• To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

• Estimator class for fit and evaluate
• Evenhandler class for callbacks in fit methods
• Unit tests
• Integration/nightly tests

Comments

• If this change is a backward incompatible change, why must this change be made.
• Interesting edge cases to note here

[marcoabreu]

Would it be possible to make these tests part of the training test suite instead of introducing new jobs?

[roywei]

@marcoabreu are you referring to training test suite here? we want to only test this on nightly. Please point to the correct test suite if I m wrong. Thanks!

[nswamy]

Suggest to move it to contrib as there is some feedback from Mu on the dev@. We could also gather feedback from the users to see what other changes are required.

Could you please break all the backlog items into Jira tasks and paste the master ticket to this PR ? Any contributor interested to further contribute to this could pick up those tasks.

[roywei]

@nswamy done, all JIRA tickets has detailed description and reference to the feedback (either from cwiki or dev list discussion)

[szha]

please break this down into different event classes and use the same approach as gluon's forward hook through weakref. this has the benefit of:

• people can mix what they need into a unified handler through multi-inheritence, without the unnecessary pass calls.
• a handler can be detached at will.

[roywei]

i added a method to check if a handlers has implemented train_begin ect to avoid unnecessary pass calls. The time it takes should be the same as using multi-inheritence and do a bunch of isinstance() at the beginning. The benefit is user can override any event call without inherit multiple class.

Still looking into how to use forward hook to register different input args

Thanks!

[szha]

Thanks for the contribution. see above. more comments to come.

[piyushghai]

Thanks for your contributions @roywei.
This is a very useful API for users and it simplifies a lot of pain points.

@mxnet-label-bot Add [Gluon, pr-awaiting-review]

[roywei]

Thanks @szha for the feedback! I m noting the summary of offlien discussion here:

1. Currently event handlers has access to entire estimator. Estimator has to maintain different stats/states (e.g. current epoch, num of steps, metrics) and do the book keeping to ensure eventhandlers has them when they are called.
2. If user want to add an event handler that want to use some info estimator does not have. He has to add it in estimator and change the estimator code to do book keeping. He has to know he can access current epoch from estimator.currrent_epoch, not some other variable name.
3. Separate event handlers into 6 classes (train begin class, train end class ect) instead of single eventhandler parent class. Each class maintain it's own info/state and know what args to pass when called. So estimator does not do the state management.
4. Using the gluon forward hook approch to provide avaibilty to detach a event handler. (e.g. remove some handler after n epochs/steps)

[nswamy]

I am not sure I understand the concerns,

What is the problem with 1) ?

For 2), the user can create custom event Handler taking objects whatever it needs to keep track of.

MyEventHandler
def __init__(self, whatever1, whatever2):
     self.whatever1 = whatever1, ...

def train_begin():
     # dowhatever using self.whatever1

Given that the fit API is targeted at novice users, I think 3) is going to make it unnecessarily cumbersome.

What is the benefit of using the Forward Hook approach?

[nswamy]

wouldn't it be easier for the user to write the training loop if they want more control instead of having the loop split 6 or more methods or hooks.
In my opinion trying to make it more flexible would complicate the usage and add to the users cognitive load.

[nswamy]

blocking the PR until my questions are answered.

I would like to understand why and how Sheng's proposal is better than the current design which was discussed offline and surfaced on dev@ months ago.
Last minute requests to fundamentally change the design should have very strong reasons.

[roywei]

@nswamy @szha I have addressed the concerns on callback in this doc: https://cwiki.apache.org/confluence/display/MXNET/Callback+Design+for+Fit+Loop
and created a PR here: #14685

Please help take a look, thanks!

[roywei]

@eric-haibin-lin @pinaraws @szha Thanks for the review, address comments here: https://github.com/apache/incubator-mxnet/pull/14885/files

[eric-haibin-lin]

My comments are addressed. Great work!!

[roywei]

@szha @eric-haibin-lin CI finally passed, validation/miscellaneous job status returned not correctly (passed instead of pending). Could you help merge if looks good?

[szha]

Nice work! Great job upholding the quality even at the cost of several iterations. Well done!