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.

Sign up for GitHub

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

Jump to bottom

Extend Scorer #1475

Merged
merged 22 commits into from
May 24, 2022
Merged

Extend Scorer #1475

merged 22 commits into from
May 24, 2022

Conversation

KEggensperger
Copy link
Contributor

@KEggensperger KEggensperger commented May 13, 2022 •
edited
Loading

This PR enables ASKL to work with a wider range of metrics and scorers. Concretely, it does the following:

  • Allow a metric to require xdata
  • Store xdata and pass it to the metric if required
  • Updates to the backend in automl-common to store and load xdata
  • Some fixed typos in the contribution guide

@codecov
Copy link

codecov bot commented May 13, 2022 •
edited
Loading

Codecov Report

Merging #1475 (26ff4e5) into development (becbd07) will decrease coverage by 0.05%.
The diff coverage is 69.56%.

@@               Coverage Diff               @@
##           development    #1475      +/-   ##
===============================================
- Coverage        84.33%   84.27%   -0.06%     
===============================================
  Files              151      151              
  Lines            11437    11488      +51     
  Branches          1984     1994      +10     
===============================================
+ Hits              9645     9682      +37     
- Misses            1265     1275      +10     
- Partials           527      531       +4

Impacted file tree graph

mfeurer
mfeurer reviewed May 16, 2022
View reviewed changes
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.

This looks good in general, but it would be great if this would be accompanied by some unit tests, for example for the train evaluator and the metrics module.

autosklearn/evaluation/train_evaluator.py Outdated Show resolved Hide resolved
autosklearn/evaluation/train_evaluator.py Outdated Show resolved Hide resolved
eddiebergman
eddiebergman approved these changes May 16, 2022
View reviewed changes
autosklearn/metrics/__init__.py


def make_scorer(
name: str,
score_func: Callable,
*,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

examples/40_advanced/example_metrics.py Outdated
def metric_which_needs_x(solution, prediction, x_data, extra_argument):
# custom function defining accuracy
assert x_data is not None
return np.mean(solution[extra_argument, :] == prediction[extra_argument, :])
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we illustrate with some arbitrary use of x_data for the example, i.e. make rows with NaNs count for twice as much?

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 updated the example with a more meaningful metric

@mfeurer mfeurer mentioned this pull request May 17, 2022
Merged
mfeurer
mfeurer reviewed May 20, 2022
View reviewed changes
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.

This looks all good, a new unit test in the file test_train_evaluator.py, which ensures that the data is correctly emitted to disk, would be great to have, though.

autosklearn/metrics/__init__.py Outdated Show resolved Hide resolved
autosklearn/metrics/__init__.py Outdated Show resolved Hide resolved
autosklearn/metrics/__init__.py Outdated Show resolved Hide resolved
autosklearn/metrics/__init__.py Outdated Show resolved Hide resolved
autosklearn/metrics/__init__.py Outdated Show resolved Hide resolved
examples/40_advanced/example_metrics.py Outdated Show resolved Hide resolved
@eddiebergman
Copy link
Contributor

I will run the benchmark on this, it's hard to implement this feature into the ensemble_builder until this is merged into dev.

@KEggensperger @mfeurer
Update examples/40_advanced/example_metrics.py
26ff4e5 
Co-authored-by: Matthias Feurer <feurerm@informatik.uni-freiburg.de>
@eddiebergman eddiebergman merged commit 4b21134 into development May 24, 2022
github-actions bot pushed a commit that referenced this pull request May 24, 2022
Katharina Eggensperger: Extend Scorer (#1475)
669e4b3
@eddiebergman eddiebergman linked an issue Jun 10, 2022 that may be closed by this pull request
Custom Metric---passing independent variable during cross validation #465
Closed
@eddiebergman eddiebergman mentioned this pull request Jun 10, 2022
Closed
eddiebergman pushed a commit that referenced this pull request Aug 18, 2022
@KEggensperger @mfeurer @eddiebergman
Extend Scorer (#1475)
a8a3cae 
* FIX minor

* update submodule

* update submodule

* ADD pass xdata to metric

* update submodule

* Fix tests

* update submodule

* ADD example

* UPDATE example

* ADD extra method to concat data

* RM note

* Fix minor

* ADD more types for xdata

* FIX unittests

* FIX variable naming bug

* change varible name; fix docstring

* Rename variable

* FIX example

* Update examples/40_advanced/example_metrics.py

Co-authored-by: Matthias Feurer <feurerm@informatik.uni-freiburg.de>

Co-authored-by: Matthias Feurer <feurerm@informatik.uni-freiburg.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfeurer mfeurer mfeurer approved these changes

@eddiebergman eddiebergman eddiebergman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Custom Metric---passing independent variable during cross validation
3 participants
@KEggensperger @eddiebergman @mfeurer