search

georgian-io-archive / foreshadow

An automatic machine learning system
https://foreshadow.readthedocs.io
Apache License 2.0
29 stars 2 forks source link

Ractor the estimator to estimator wrapper. #187

Closed jzhang-gp closed 4 years ago

jzhang-gp commented 4 years ago

Description

The current estimator in foreshadow is a MetaEstimator instance, which wraps over the user specified estimator or the default AutoEstimator instance. However, to access the actual estimator, users have to use code like foreshadow.estimator.estimator or foreshadow.estimator.estimator.estimator (when using AutoEstimator) even though when creating the foreshadow instance, it is foreshadow = Foreshadow(estimator=YOUR_FAVORITE_ESTIMATOR_INSTANCE). This creates an inconsistency on the expectation.

This change adds an estimator_wrapper attribute and prevents users from accessing the estimator in the old way.

The ration behind is that if we have to expose the internal structure of Foreshadow's estimator, at least be explicit that the true estimator is wrapped.

Another possible solution is to return the actual estimator user specified or the AutoEstimator instance when accessing the estimator attribute. In fact, while I'm writing this down, I feel this might be a better solution to further align user expectations. Let me know your thoughts. Thanks!

jzhang-gp commented 4 years ago

It turns out that I've already implemented the better solution. I probably forgot I have done that since it was done more than a week ago...Anyway, I've updated the unit test to verify that user can access the estimator.