Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix moo things #1501

Merged
merged 13 commits into from
Jun 14, 2022
Merged

Fix moo things #1501

merged 13 commits into from
Jun 14, 2022

Conversation

eddiebergman
Copy link
Contributor

@eddiebergman eddiebergman commented Jun 8, 2022


Fixes:

  • Bug from ensemble builder where there performance history was appended twice, see comment marked below with BUGFIX

Tests (Tangent):

  • You can ignore test_post_fit and test_performance, just reorganizing tests
  • Remove test_output.py and just merge it with test_post_fit.py since theyre mostly similar
  • Add comment header to test_automl/ files to describe them more

Next Steps, will create issues and work on it once this has been merged:

  • The ensemble history only returns one metric from builder as this requires reworking AutoML::performance_over_time. Thought this is best left for another PR
  • We need some kind of basic multiobjective implementation to actually test things. Could even be a dummy one that only exists as a minimal implementation for the sake of testing.

@eddiebergman eddiebergman changed the base branch from master to development June 8, 2022 11:09
@codecov
Copy link

codecov bot commented Jun 8, 2022

Codecov Report

Merging #1501 (55c792b) into development (0f1f38a) will decrease coverage by 0.01%.
The diff coverage is 62.06%.

@@               Coverage Diff               @@
##           development    #1501      +/-   ##
===============================================
- Coverage        83.79%   83.77%   -0.02%     
===============================================
  Files              152      152              
  Lines            11667    11683      +16     
  Branches          2037     2044       +7     
===============================================
+ Hits              9776     9788      +12     
- Misses            1343     1344       +1     
- Partials           548      551       +3     

Impacted file tree graph

Comment on lines +630 to +632

# Add the performance stamp to the history
self.ensemble_history.append(performance_stamp)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BUGFIX: Marking this to match PR description, used to be inside the for loop but should have been outside

@eddiebergman eddiebergman requested a review from mfeurer June 9, 2022 19:01
@eddiebergman eddiebergman linked an issue Jun 10, 2022 that may be closed by this pull request
@eddiebergman eddiebergman self-assigned this Jun 10, 2022
@eddiebergman eddiebergman added this to the V0.15 milestone Jun 10, 2022
@eddiebergman eddiebergman added the maintenance Internal maintenance label Jun 10, 2022
@mfeurer mfeurer merged commit ad0675f into development Jun 14, 2022
@mfeurer mfeurer deleted the fix_moo_things branch June 14, 2022 12:34
github-actions bot pushed a commit that referenced this pull request Jun 14, 2022
eddiebergman added a commit that referenced this pull request Aug 18, 2022
* Push

* `fit_ensemble` now has priority for kwargs to take

* Change ordering of prefernce for ensemble params

* Add TODO note for metrics

* Add `metrics` arg to `fit_ensemble`

* Add test for pareto front sizes

* Remove uneeded file

* Re-added tests to `test_pareto_front`

* Add descriptions to test files

* Add test to ensure argument priority

* Add test to make sure X_data only loaded when required

* Remove part of test required for performance history

* Default to `self._metrics` if `metrics` not available
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment