add SklearnCallback as integration with sklearn metrics #1198

y-ksenia · 2021-04-29T21:24:29Z

Before submitting (checklist)

Description

Work on #1183

Related Issue

Type of Change

Examples / docs / tutorials / contributors update
Bug fix (non-breaking change which fixes an issue)
Improvement (non-breaking change which improves an existing feature)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

FAQ

Please review the FAQ before submitting an issue:

I have read the documentation and FAQ
I have reviewed the minimal examples section
I have checked the changelog for main framework updates
I have read the contribution guide
I have joined Catalyst slack (#__questions channel) for issue discussion

Scitator

could you please integrate the Callback to a minimal example to check the performance?

catalyst/callbacks/metrics/sklearn.py

Scitator · 2021-04-30T05:19:41Z

catalyst/callbacks/metrics/sklearn.py

+from catalyst.metrics import FunctionalBatchMetric
+
+
+class SklearnCallback(FunctionalBatchMetricCallback):


is it possible to also add loader-based sklearn metrics into the separate Callback?
we could not compute AUCs in per-batch mode

nevertheless, we could just filter the loader-based metrics during metric_fn check for now
to simplify the PR

should I filter by name?.. or how?

catalyst/callbacks/metrics/__init__.py

catalyst/callbacks/metrics/scikit_learn.py

ditwoo · 2021-05-13T16:15:09Z

@y-ksenia btw you can test SklearnCallback emulating experiment flow (without executing the whole experiment).

Scitator

could you please also update Config API example with extra SklearnCallback?

Scitator · 2021-05-14T18:10:53Z

tests/catalyst/tests/test_finetune2.py

+            callbacks["f1_score"] = dl.SklearnCallback(
+                keys={
+                    "y_pred": "labels",
+                    "y_true": "targets",
+                    "average": "micro",
+                    "zero_division": 1,
+                },
+                metric_fn="f1_score",
+                metric_key="f1_score",
+            )


could you please add this example to the callbacks docs? it's very interesting, that you could also pass metrics **kwargs through the keys

catalyst/callbacks/metrics/scikit_learn.py

mergify · 2021-05-18T20:12:08Z

This pull request is now in conflicts. @y-ksenia, could you fix it? 🙏

…s to sklearn score

…way of passing arguments into metric_fn

pep8speaks · 2021-05-26T12:05:37Z

Hello @y-ksenia! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-05-30 20:39:17 UTC

Scitator

looks like only changelog.md update required :)

docs/api/callbacks.rst

Pull request has been modified.

mergify · 2021-05-30T20:18:07Z

This pull request is now in conflicts. @y-ksenia, could you fix it? 🙏

* Update CHANGELOG.md * Add Registry to batch transform callback (#1209) * add registry * fix docs * fix docs and codestyle. Add example * fix line length :) * add registry.py * codestyle * fix * add partial and kwargs * Fix data: imports (#1211) * notebooks update v1 * notebooks update v2 * notebooks update v2 * notebooks update v2 * minimal RL * minimal RL * minimal RL 2 * rl update * rl update * demo update * demo update * demo update * demo update * tests update * tests update * import fix * readme * Change layerwise to layerwise_params (#1210) * Change layerwise to layerwise_params (rus version) At the old version was incorrect key layerwise to set lr, weight_decay and other params for different model layers. Also add example how to add extra params to DataLoader * Change layerwise to layerwise_params (eng version) At the old version was incorrect key layerwise to set lr, weight_decay and other params for different model layers. Also add example how to add extra params to DataLoader * [WIP] faq docs (#1202) * initial * docs: engines initial * docs: engines initial * fix: python api; feature: config api example * docs: reverted back * docs: spelling; engine functions explained * fix: note in changelog about faq docs * Update CHANGELOG.md (#1216) * fix: to_numpy wrapper for `AdditiveValueMetric` added (#1214) * tests update + readme (#1215) * tests update + readme * readme * workflows changed * workflows * readme * Fix compute method (#1206) * add DynamicBalanceClassSampler * add DynamicBalanceClassSampler: add usage example * add DynamicBalanceClassSampler: add tests * Update catalyst/data/tests/test_sampler.py * Update catalyst/data/tests/test_sampler.py * add DynamicBalanceClassSampler: debag tests * update sampler: add mode * add example notebook * sampler: fixes * samler: docs * DynamicBalanceClassSampler: fixes * change import order * change import order * Fix compute problem * Delete sampler * Replace compute method * Update segmentation tests * Codestyle * Add test for compute * Add examples Co-authored-by: Sergey Kolesnikov <scitator@gmail.com> * Update ddt.rst (#1217) Change in Case-3 code, Now running the code under name==__main__ (due to python's multiprocessing logic) * docker update (#1218) * docker update * docker update * docs update (#1219) * Updated NeptuneLogger docstrings (#1223) * fixed import in example, changed .ml -> .ai * fixed long lines, added NeptuneLogger Callback * fixed NeptuneLogger imports * fixed newline formating * fixed line length and import order * fixed bad quotes * fixed bad quotes * vanilla impl [WIP] * fixed hparams * applied new structure to run * corrected log image path * added log_artifact to catalyst * Updated log_artifact docstring * simplified logger, added log artifact * log to 'experiment' scope is scope is None * fixed artifact logging * better structure of the run * code style adjustments * added neptune requirements to the workflow * typo fix * dropped repetition in requirements * codestyle fixes * filename fix * Corrected docstring to be 1-liner. * Update catalyst/loggers/neptune.py Co-authored-by: Sergey Kolesnikov <scitator@gmail.com> * Final touches & test fixes * Checkstyle fixes * Missed code style * Revert stop to wait * Fix failing test * Comment indentation * fix tests * Fix * Fix * fix * Fix * Updated CHANGELOG and loggers docs * Fix * Update setup.py Co-authored-by: Sergey Kolesnikov <scitator@gmail.com> * updated neptune docstrings * style fixed * updated changelog * typo fix Co-authored-by: Jakub Czakon <czakon.jakub@gmail.com> Co-authored-by: Sergey Kolesnikov <scitator@gmail.com> Co-authored-by: Hubert Jaworski <hubert.jaworski@neptune.ai> Co-authored-by: Marcin Mycek <herudaio@gmail.com> * remove unnecessary thing * Update catalyst/callbacks/batch_transform.py * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Sergey Kolesnikov <scitator@gmail.com> Co-authored-by: MrNightSky <39031563+MrNightSky@users.noreply.github.com> Co-authored-by: Dmytro Doroshenko <dimdoroshenko@gmail.com> Co-authored-by: Yauheni Kachan <19803638+bagxi@users.noreply.github.com> Co-authored-by: Dokholyan <37542142+Dokholyan@users.noreply.github.com> Co-authored-by: Riyasat Ohib <riyasat.ohib@gmail.com> Co-authored-by: Kamil A. Kaczmarek <kamil.kaczmarek@neptune.ml> Co-authored-by: Jakub Czakon <czakon.jakub@gmail.com> Co-authored-by: Hubert Jaworski <hubert.jaworski@neptune.ai> Co-authored-by: Marcin Mycek <herudaio@gmail.com> * add SklearnCallback as integration with sklearn metrics (#1198) * added SklearnCallback as integration with sklearn metrics * codestyle * renamed script to exclude error in imports; added option to add params to sklearn score * added import if sklearn is available in extensions * added test * docstrings * updated docstring: added example for pass metrics kwargs as keys * added note to callback * added FunctionalLoaderMetric and SklearnLoaderCallback; modified the way of passing arguments into metric_fn * codestyle fixes * fixed test for new version of SklearnBatchCallback * added __all__ * added classes to docs * codestyle * fixed docs; and small pep8-bugs * updated changelog * codestyle * codestyle2 Co-authored-by: Nikita Balagansky <37884009+elephantmipt@users.noreply.github.com> Co-authored-by: MrNightSky <39031563+MrNightSky@users.noreply.github.com> Co-authored-by: Dmytro Doroshenko <dimdoroshenko@gmail.com> Co-authored-by: Yauheni Kachan <19803638+bagxi@users.noreply.github.com> Co-authored-by: Dokholyan <37542142+Dokholyan@users.noreply.github.com> Co-authored-by: Riyasat Ohib <riyasat.ohib@gmail.com> Co-authored-by: Kamil A. Kaczmarek <kamil.kaczmarek@neptune.ml> Co-authored-by: Jakub Czakon <czakon.jakub@gmail.com> Co-authored-by: Hubert Jaworski <hubert.jaworski@neptune.ai> Co-authored-by: Marcin Mycek <herudaio@gmail.com> Co-authored-by: Kseniia Iagafarova <xeniayagafarova@gmail.com>

y-ksenia requested review from bagxi, ditwoo and Scitator as code owners April 29, 2021 21:24

y-ksenia changed the title ~~added SklearnCallback as integration with sklearn metrics~~ add SklearnCallback as integration with sklearn metrics Apr 29, 2021

Scitator reviewed Apr 30, 2021

View reviewed changes

y-ksenia force-pushed the sklearn_callback branch from 259295c to 27d3c46 Compare May 4, 2021 16:49

Scitator reviewed May 5, 2021

View reviewed changes

catalyst/callbacks/metrics/__init__.py Outdated Show resolved Hide resolved

catalyst/callbacks/metrics/scikit_learn.py Outdated Show resolved Hide resolved

y-ksenia force-pushed the sklearn_callback branch from cb6e1fd to 4fdfc56 Compare May 12, 2021 21:02

Scitator reviewed May 14, 2021

View reviewed changes

Scitator reviewed May 17, 2021

View reviewed changes

catalyst/callbacks/metrics/scikit_learn.py Show resolved Hide resolved

y-ksenia added 7 commits May 25, 2021 16:49

added SklearnCallback as integration with sklearn metrics

fd917f4

codestyle

aae2046

renamed script to exclude error in imports; added option to add param…

7e73c5e

…s to sklearn score

added import if sklearn is available in extensions

7deb5e5

added test

e2359c4

docstrings

2e52ee5

updated docstring: added example for pass metrics kwargs as keys

50525a5

y-ksenia force-pushed the sklearn_callback branch from dc2b091 to 50525a5 Compare May 25, 2021 13:49

y-ksenia added 7 commits May 25, 2021 16:51

added note to callback

c23e0b3

added FunctionalLoaderMetric and SklearnLoaderCallback; modified the …

5c18e16

…way of passing arguments into metric_fn

codestyle fixes

270d3f5

fixed test for new version of SklearnBatchCallback

ef3b289

added __all__

f3725c1

added classes to docs

0b0e690

codestyle

5a5ee4a

Scitator self-requested a review May 26, 2021 17:28

Scitator previously approved these changes May 26, 2021

View reviewed changes

docs/api/callbacks.rst Outdated Show resolved Hide resolved

fixed docs; and small pep8-bugs

4f85fdc

updated changelog

0dba4be

Merge branch 'master' into sklearn_callback

c45a287

Scitator changed the base branch from master to develop May 31, 2021 05:40

Scitator merged commit 8bcf87d into catalyst-team:develop May 31, 2021

y-ksenia deleted the sklearn_callback branch June 1, 2021 09:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add SklearnCallback as integration with sklearn metrics #1198

add SklearnCallback as integration with sklearn metrics #1198

y-ksenia commented Apr 29, 2021 •

edited

Scitator left a comment

Scitator Apr 30, 2021

Scitator Apr 30, 2021

y-ksenia May 13, 2021

ditwoo commented May 13, 2021

Scitator left a comment

Scitator May 14, 2021

mergify bot commented May 18, 2021

pep8speaks commented May 26, 2021 •

edited

Scitator left a comment

mergify bot commented May 30, 2021

		from catalyst.metrics import FunctionalBatchMetric


		class SklearnCallback(FunctionalBatchMetricCallback):

add SklearnCallback as integration with sklearn metrics #1198

add SklearnCallback as integration with sklearn metrics #1198

Conversation

y-ksenia commented Apr 29, 2021 • edited

Before submitting (checklist)

Description

Related Issue

Type of Change

PR review

FAQ

Scitator left a comment

Choose a reason for hiding this comment

Scitator Apr 30, 2021

Choose a reason for hiding this comment

Scitator Apr 30, 2021

Choose a reason for hiding this comment

y-ksenia May 13, 2021

Choose a reason for hiding this comment

ditwoo commented May 13, 2021

Scitator left a comment

Choose a reason for hiding this comment

Scitator May 14, 2021

Choose a reason for hiding this comment

mergify bot commented May 18, 2021

pep8speaks commented May 26, 2021 • edited

Comment last updated at 2021-05-30 20:39:17 UTC

Scitator left a comment

Choose a reason for hiding this comment

mergify bot commented May 30, 2021

y-ksenia commented Apr 29, 2021 •

edited

pep8speaks commented May 26, 2021 •

edited