Skip to content

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