Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Extend estimator.evaluate() to support event handlers #16971

Merged
merged 7 commits into from
Dec 10, 2019

Conversation

liuzh47
Copy link
Contributor

@liuzh47 liuzh47 commented Dec 4, 2019

Description

Motivated by the issue #16959, we find that we could use existing LoggingHandler and MetricHandler for validation purpose. We extend estimator.evaluate() to support default and customized handlers. Our contributions can be summarized below:

  • We extend estimator.evaluate() to support event_handlers
  • We replace the metrics in estimator constructor with train_metrics and val_metrics
  • We decouple the dependence of default event handlers on training or validation metrics
  • We add test script to verify the correctness of evaluate function

Fix #16959.

@liuzh47 liuzh47 requested a review from szha as a code owner December 4, 2019 10:46
Copy link
Contributor

@leezu leezu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. LGTM. Please add comments to the code for below two points to make the life of future code readers easier.

@liuzh47 liuzh47 force-pushed the loggingeval branch 2 times, most recently from dd8d725 to ebd8c67 Compare December 9, 2019 03:38
@leezu leezu merged commit 0c17ddd into apache:master Dec 10, 2019
@ptrendx
Copy link
Member

ptrendx commented Dec 10, 2019

@leezu - do you also want this in 1.6? I'm cherry-picking the other stuff, might as well take this since it is merged.

@leezu
Copy link
Contributor

leezu commented Dec 10, 2019

@ptrendx, yes, that would be great

ptrendx pushed a commit to ptrendx/mxnet that referenced this pull request Dec 11, 2019

* fix unittest failures for the new api interface

* Add comments in the code for readability

* Remove unused argument val_metrics

* merge changes with the master branch

* fix some regression errors

* fix bugs introduced in the merging phase
ptrendx added a commit that referenced this pull request Dec 11, 2019
* Include eval_net the validation model in the gluon estimator api (#16957)

* Include eval_net the validation model in the estimator api

* fix small issue

* Extend estimator.evaluate() to support event handlers (#16971)



* fix unittest failures for the new api interface

* Add comments in the code for readability

* Remove unused argument val_metrics

* merge changes with the master branch

* fix some regression errors

* fix bugs introduced in the merging phase

* Add support of plug and play fit_batch and evaluate_batch (#16982)

* Add support of plug and play fit_batch and evaluate_batch

* Add check for the validity of the estimator model

* Rename estimator model as batch processor

* Remove unused import

* Add documentation of the batch processor class

* refine the documentation of the batch processor

* Fix merge bugs

* fix bugs introduced during merge

* fix sanity check failures

* fix CI bugs

* Fix Gluon Estimator nightly test (#17042)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Estimator.evaluate does not support logging
3 participants