Skip to content

Conversation

franchuterivera
Copy link
Contributor

This pull request removes the competition manager. Its current usage is:

  • In automl for fit automl dataset
  • In smbo for loading data

So I removed the fit automl dataset function, aligning to only use the xy datamanager

In smbo, the load function would have become a call to backed load data. I removed the load_data function, which was a straight forward change except on the test side. In here I movedto loading standart datasets using sklearn functions.

self.logging_config = logging_config
self.metadata_directory = metadata_directory

self._automl = None # type: Optional[List[BaseAutoML]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, what's the reason to drop this? Was it incorrect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the comment here. The logic behind this was:

There was an unnecessary import here to BaseAutoML. I thought it was a legacy due to the comment:
self._automl = None # type: Optional[List[BaseAutoML]

Here, the automl attribute defaults to None. The comment doesn't make sense? Please let me know if you still want to keep that comment!

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, that's a type hint: https://docs.python.org/3/library/typing.html

We don't use them as much as we should yet, but I'd like to keep it.

D = CompetitionDataManager(dataset_path)
# https://www.openml.org/d/183
dataset_name = 'abalone'
task = 'multiclass.classification'
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn' this be a constant from here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you have a look at this?

self.logging_config = logging_config
self.metadata_directory = metadata_directory

self._automl = None # type: Optional[List[BaseAutoML]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, that's a type hint: https://docs.python.org/3/library/typing.html

We don't use them as much as we should yet, but I'd like to keep it.

self._resampling_strategy = resampling_strategy
self._resampling_strategy_arguments = resampling_strategy_arguments \
if resampling_strategy_arguments is not None else {}
if self._resampling_strategy not in ['holdout',
Copy link
Contributor

Choose a reason for hiding this comment

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

These were so far part of the fit method, is there a reason you moved them to init?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If they are provided in the estimator definition, shouldn't the check be done there?
Because if someone defines an incorrect resampling strategy when doing the estimator, he will find out during fit, but then, he will have to redefine the estimator?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, true, but then there are a lot of other checks within fit that you did not move. Could you please move them for consistency?

@mfeurer
Copy link
Contributor

mfeurer commented Jun 12, 2020

Not sure if this PR was ready for review, but could you please have a look at the unit tests, too?

@franchuterivera
Copy link
Contributor Author

Just one last clarification regarding the typing. As it is, it causes a imported but unused flake error, and that is the reason I originally deleted it.

It currently is like:
self._automl = None # type: Optional[List[BaseAutoML]]

Is it ok to add an automl argument to the constructor, properly define the typing there and default it to none?

@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2020

Codecov Report

Merging #869 into development will increase coverage by 0.48%.
The diff coverage is 81.48%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #869      +/-   ##
===============================================
+ Coverage        84.06%   84.55%   +0.48%     
===============================================
  Files              127      126       -1     
  Lines             9435     9218     -217     
===============================================
- Hits              7932     7794     -138     
+ Misses            1503     1424      -79     
Impacted Files Coverage Δ
autosklearn/automl.py 81.93% <80.39%> (+0.17%) ⬆️
autosklearn/estimators.py 90.41% <100.00%> (+0.05%) ⬆️
autosklearn/smbo.py 72.72% <100.00%> (-0.70%) ⬇️
autosklearn/data/abstract_data_manager.py 77.02% <0.00%> (-12.17%) ⬇️
autosklearn/evaluation/__init__.py 80.54% <0.00%> (-2.17%) ⬇️
autosklearn/ensemble_builder.py 69.89% <0.00%> (-1.19%) ⬇️
autosklearn/evaluation/train_evaluator.py 72.62% <0.00%> (+0.45%) ⬆️
...eline/components/feature_preprocessing/fast_ica.py 97.82% <0.00%> (+6.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d313f26...b460564. Read the comment docs.

D = CompetitionDataManager(dataset_path)
# https://www.openml.org/d/183
dataset_name = 'abalone'
task = 'multiclass.classification'
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you have a look at this?

@mfeurer
Copy link
Contributor

mfeurer commented Jun 13, 2020

Is it ok to add an automl argument to the constructor, properly define the typing there and default it to none?
Yes, totally

Copy link
Contributor

@mfeurer mfeurer left a comment

Choose a reason for hiding this comment

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

Hey, this looks really good, but I think there's at least one more necessary change.

@franchuterivera franchuterivera force-pushed the remove_competition_loader branch from 5bb86d2 to b460564 Compare June 13, 2020 10:12
@mfeurer mfeurer merged commit 47a3f12 into automl:development Jun 15, 2020
charlesfu4 pushed a commit to charlesfu4/auto-sklearn that referenced this pull request Jun 17, 2020
* Removal of competition manager

* Removed additional unused methods/files and moved metrics to estimator

* Fix meta data generation

* Make sure pytest is older newer than 4.6

* Unit tst fixing

* flake8 fixes in examples

* Fix metadata gen metrics
charlesfu4 pushed a commit to charlesfu4/auto-sklearn that referenced this pull request Jun 17, 2020
* Removal of competition manager

* Removed additional unused methods/files and moved metrics to estimator

* Fix meta data generation

* Make sure pytest is older newer than 4.6

* Unit tst fixing

* flake8 fixes in examples

* Fix metadata gen metrics
mfeurer added a commit that referenced this pull request Jul 3, 2020
* PEP8 (#718)

* multioutput_regression

* multioutput_regression

* multioutput_regression

* multioutput regression

* multioutput regression

* multioutput regression

* multioutput regression

* multioutput regression

* #782 showcase pipeline components iteration

* Fixed flake-8 violations

* multi_output regression v1

* fix y_shape in multioutput regression

* fix xy_data_manager change due to merge

* automl.py missing import

* Release note 070 (#842)

* First version of 070 release notes

* Missed a bugfix

* Vim added unexpected space -- fix

* prepare new release (#846)

* Clip predict values to [0-1] in classification

* Fix for 3.5 python!

* Sensible default value of 'score_func' for SelectPercentileRegression (#843)

Currently default value of 'score_func' for SelectPercentileRegression
is "f_classif", which is an invalid value, and will surely be rejected and
will not work

* More robust tmp file naming (#854)

* More robust tmp file naming

* UUID approach

* 771 worst possible result (#845)

* Initial Commit

* Make worst result a function

* worst possible result in metric

* Fixing the name of the scorers

* Add exceptions to log file, not just stdout (#863)

* Add exceptions to log file, not just stdout

* Removing dummy pred as trys is not needed

* Add prediction with models trained with cross-validation (#864)

* add the possibility to predict with cross-validation

* fix unit tests

* test new feature, too

* 715 ml memory (#865)

* #715 Support for no ml memory limit

* API update

* Docs enhancement (#862)

* Improved docs

* Fixed example typos

* Beautify examples

* cleanup examples

* fixed rsa equal

* Move to minmax scaler (#866)

* Do not read predictions in memory, only after score (#870)

* Do not read predictions in memory, only after score

* Precission support for string/int

* Removal of competition manager (#869)

* Removal of competition manager

* Removed additional unused methods/files and moved metrics to estimator

* Fix meta data generation

* Make sure pytest is older newer than 4.6

* Unit tst fixing

* flake8 fixes in examples

* Fix metadata gen metrics

* Fix dataprocessing get params (#877)

* Fix dataprocessing get params

* Add clone-test to regression pipeline

* Allow 1-D threshold binary predictions (#879)

* fix single output regression not working

* regression need no _enusre_prediction_array_size_prediction_array_sizess

* #782 showcase pipeline components iteration

* Fixed flake-8 violations

* Release note 070 (#842)

* First version of 070 release notes

* Missed a bugfix

* Vim added unexpected space -- fix

* prepare new release (#846)

* Clip predict values to [0-1] in classification

* Fix for 3.5 python!

* Sensible default value of 'score_func' for SelectPercentileRegression (#843)

Currently default value of 'score_func' for SelectPercentileRegression
is "f_classif", which is an invalid value, and will surely be rejected and
will not work

* More robust tmp file naming (#854)

* More robust tmp file naming

* UUID approach

* 771 worst possible result (#845)

* Initial Commit

* Make worst result a function

* worst possible result in metric

* Fixing the name of the scorers

* Add exceptions to log file, not just stdout (#863)

* Add exceptions to log file, not just stdout

* Removing dummy pred as trys is not needed

* Add prediction with models trained with cross-validation (#864)

* add the possibility to predict with cross-validation

* fix unit tests

* test new feature, too

* 715 ml memory (#865)

* #715 Support for no ml memory limit

* API update

* Docs enhancement (#862)

* Improved docs

* Fixed example typos

* Beautify examples

* cleanup examples

* fixed rsa equal

* Move to minmax scaler (#866)

* Do not read predictions in memory, only after score (#870)

* Do not read predictions in memory, only after score

* Precission support for string/int

* Removal of competition manager (#869)

* Removal of competition manager

* Removed additional unused methods/files and moved metrics to estimator

* Fix meta data generation

* Make sure pytest is older newer than 4.6

* Unit tst fixing

* flake8 fixes in examples

* Fix metadata gen metrics

* Fix dataprocessing get params (#877)

* Fix dataprocessing get params

* Add clone-test to regression pipeline

* Allow 1-D threshold binary predictions (#879)

* multioutput_regression

* multioutput_regression

* multioutput_regression

* multioutput_regression

* multioutput_regression

* multioutput_regression

* multioutput regression

* multioutput regression

* multioutput regression

* multioutput regression

* multi_output regression v1

* fix y_shape in multioutput regression

* fix xy_data_manager change due to merge

* fix single output regression not working

* regression need no _enusre_prediction_array_size_prediction_array_sizess

* Add prediction with models trained with cross-validation (#864)

* add the possibility to predict with cross-validation

* fix unit tests

* test new feature, too

* multioutput_regression

* multioutput_regression

* multioutput_regression

* Removal of competition manager (#869)

* Removal of competition manager

* Removed additional unused methods/files and moved metrics to estimator

* Fix meta data generation

* Make sure pytest is older newer than 4.6

* Unit tst fixing

* flake8 fixes in examples

* Fix metadata gen metrics

* multioutput after rebased to 0.7.0

Problem:

Cause:

Solution:

* Regressor target y shape index out of range

* Revision for make tester

* Revision: Cancel Multiclass-MultiOuput

* Resolve automl.py metrics(__init__) reg_gb reg_svm

* Fix Flake8 errors

* Fix automl.py flake8

* Preprocess w/ mulitout reg,automl self._n_outputs

* test_estimator.py changed back

* cancel multioutput multiclass for multi reg

* Fix automl self._n_output update placement

* fix flake8

* Kernel pca cancelled mulitout reg

* Kernel PCA test skip python <3.8

* Add test unit for multioutput reg and fix.

* Fix flake8 error

* Kernel PCA multioutput regression

* default kernel to cosine, dodge sklearn=0.22 error

* Kernel PCA should be updated to 0.23

* Kernel PCA uses rbf kernel

* Kernel Pca

* Modify labels in reg, class, perpro in examples

* Kernel PCA

* Add missing supports to mincoal and truncateSVD

Co-authored-by: Matthias Feurer <[email protected]>
Co-authored-by: chico <[email protected]>
Co-authored-by: Francisco Rivera Valverde <[email protected]>
Co-authored-by: Xiaodong DENG <[email protected]>
franchuterivera added a commit to franchuterivera/auto-sklearn that referenced this pull request Aug 21, 2020
* Removal of competition manager

* Removed additional unused methods/files and moved metrics to estimator

* Fix meta data generation

* Make sure pytest is older newer than 4.6

* Unit tst fixing

* flake8 fixes in examples

* Fix metadata gen metrics
franchuterivera added a commit to franchuterivera/auto-sklearn that referenced this pull request Aug 21, 2020
* PEP8 (automl#718)

* multioutput_regression

* multioutput_regression

* multioutput_regression

* multioutput regression

* multioutput regression

* multioutput regression

* multioutput regression

* multioutput regression

* automl#782 showcase pipeline components iteration

* Fixed flake-8 violations

* multi_output regression v1

* fix y_shape in multioutput regression

* fix xy_data_manager change due to merge

* automl.py missing import

* Release note 070 (automl#842)

* First version of 070 release notes

* Missed a bugfix

* Vim added unexpected space -- fix

* prepare new release (automl#846)

* Clip predict values to [0-1] in classification

* Fix for 3.5 python!

* Sensible default value of 'score_func' for SelectPercentileRegression (automl#843)

Currently default value of 'score_func' for SelectPercentileRegression
is "f_classif", which is an invalid value, and will surely be rejected and
will not work

* More robust tmp file naming (automl#854)

* More robust tmp file naming

* UUID approach

* 771 worst possible result (automl#845)

* Initial Commit

* Make worst result a function

* worst possible result in metric

* Fixing the name of the scorers

* Add exceptions to log file, not just stdout (automl#863)

* Add exceptions to log file, not just stdout

* Removing dummy pred as trys is not needed

* Add prediction with models trained with cross-validation (automl#864)

* add the possibility to predict with cross-validation

* fix unit tests

* test new feature, too

* 715 ml memory (automl#865)

* automl#715 Support for no ml memory limit

* API update

* Docs enhancement (automl#862)

* Improved docs

* Fixed example typos

* Beautify examples

* cleanup examples

* fixed rsa equal

* Move to minmax scaler (automl#866)

* Do not read predictions in memory, only after score (automl#870)

* Do not read predictions in memory, only after score

* Precission support for string/int

* Removal of competition manager (automl#869)

* Removal of competition manager

* Removed additional unused methods/files and moved metrics to estimator

* Fix meta data generation

* Make sure pytest is older newer than 4.6

* Unit tst fixing

* flake8 fixes in examples

* Fix metadata gen metrics

* Fix dataprocessing get params (automl#877)

* Fix dataprocessing get params

* Add clone-test to regression pipeline

* Allow 1-D threshold binary predictions (automl#879)

* fix single output regression not working

* regression need no _enusre_prediction_array_size_prediction_array_sizess

* automl#782 showcase pipeline components iteration

* Fixed flake-8 violations

* Release note 070 (automl#842)

* First version of 070 release notes

* Missed a bugfix

* Vim added unexpected space -- fix

* prepare new release (automl#846)

* Clip predict values to [0-1] in classification

* Fix for 3.5 python!

* Sensible default value of 'score_func' for SelectPercentileRegression (automl#843)

Currently default value of 'score_func' for SelectPercentileRegression
is "f_classif", which is an invalid value, and will surely be rejected and
will not work

* More robust tmp file naming (automl#854)

* More robust tmp file naming

* UUID approach

* 771 worst possible result (automl#845)

* Initial Commit

* Make worst result a function

* worst possible result in metric

* Fixing the name of the scorers

* Add exceptions to log file, not just stdout (automl#863)

* Add exceptions to log file, not just stdout

* Removing dummy pred as trys is not needed

* Add prediction with models trained with cross-validation (automl#864)

* add the possibility to predict with cross-validation

* fix unit tests

* test new feature, too

* 715 ml memory (automl#865)

* automl#715 Support for no ml memory limit

* API update

* Docs enhancement (automl#862)

* Improved docs

* Fixed example typos

* Beautify examples

* cleanup examples

* fixed rsa equal

* Move to minmax scaler (automl#866)

* Do not read predictions in memory, only after score (automl#870)

* Do not read predictions in memory, only after score

* Precission support for string/int

* Removal of competition manager (automl#869)

* Removal of competition manager

* Removed additional unused methods/files and moved metrics to estimator

* Fix meta data generation

* Make sure pytest is older newer than 4.6

* Unit tst fixing

* flake8 fixes in examples

* Fix metadata gen metrics

* Fix dataprocessing get params (automl#877)

* Fix dataprocessing get params

* Add clone-test to regression pipeline

* Allow 1-D threshold binary predictions (automl#879)

* multioutput_regression

* multioutput_regression

* multioutput_regression

* multioutput_regression

* multioutput_regression

* multioutput_regression

* multioutput regression

* multioutput regression

* multioutput regression

* multioutput regression

* multi_output regression v1

* fix y_shape in multioutput regression

* fix xy_data_manager change due to merge

* fix single output regression not working

* regression need no _enusre_prediction_array_size_prediction_array_sizess

* Add prediction with models trained with cross-validation (automl#864)

* add the possibility to predict with cross-validation

* fix unit tests

* test new feature, too

* multioutput_regression

* multioutput_regression

* multioutput_regression

* Removal of competition manager (automl#869)

* Removal of competition manager

* Removed additional unused methods/files and moved metrics to estimator

* Fix meta data generation

* Make sure pytest is older newer than 4.6

* Unit tst fixing

* flake8 fixes in examples

* Fix metadata gen metrics

* multioutput after rebased to 0.7.0

Problem:

Cause:

Solution:

* Regressor target y shape index out of range

* Revision for make tester

* Revision: Cancel Multiclass-MultiOuput

* Resolve automl.py metrics(__init__) reg_gb reg_svm

* Fix Flake8 errors

* Fix automl.py flake8

* Preprocess w/ mulitout reg,automl self._n_outputs

* test_estimator.py changed back

* cancel multioutput multiclass for multi reg

* Fix automl self._n_output update placement

* fix flake8

* Kernel pca cancelled mulitout reg

* Kernel PCA test skip python <3.8

* Add test unit for multioutput reg and fix.

* Fix flake8 error

* Kernel PCA multioutput regression

* default kernel to cosine, dodge sklearn=0.22 error

* Kernel PCA should be updated to 0.23

* Kernel PCA uses rbf kernel

* Kernel Pca

* Modify labels in reg, class, perpro in examples

* Kernel PCA

* Add missing supports to mincoal and truncateSVD

Co-authored-by: Matthias Feurer <[email protected]>
Co-authored-by: chico <[email protected]>
Co-authored-by: Francisco Rivera Valverde <[email protected]>
Co-authored-by: Xiaodong DENG <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants