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

Change layerwise to layerwise_params #1210

Merged
merged 2 commits into from May 16, 2021
Merged

Change layerwise to layerwise_params #1210

merged 2 commits into from May 16, 2021

Conversation

MrNightSky
Copy link
Contributor

@MrNightSky MrNightSky commented May 12, 2021 •
edited

Before submitting (checklist)

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contribution guide?
  • Did you check the code style? catalyst-make-codestyle && catalyst-check-codestyle (pip install -U catalyst-codestyle).
  • Did you make sure to update the docs? We use Google format for all the methods and classes.
  • Did you check the docs with make check-docs?
  • Did you write any new necessary tests?
  • Did you check that your code passes the unit tests pytest . ?
  • Did you add your new functionality to the docs?
  • Did you update the CHANGELOG?
  • Did you run colab minimal CI/CD with latest and minimal requirements?

Description

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:

@MrNightSky
Change layerwise to layerwise_params (rus version)
a86a12a 
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
@MrNightSky
Change layerwise to layerwise_params (eng version)
6e4b5f9 
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
@Scitator
Copy link
Member

overall... nice catch! good to see such a documentation bug found
nevertheless, @bagxi @ditwoo, what do you think about naming? layerwise_params or simple layerwise?
@MrNightSky from the usage perspective, what is more clear?

@MrNightSky
Copy link
Contributor Author

I think layerwise_params better for dict parameter than layerwise.

What's wrong with my PR? Why it not go through the test? I have not changed any code, and my changes was only at examples config, that I thought not used anywhere.

@Scitator
Copy link
Member

That's Github CI robustness issue :)

@Scitator
Copy link
Member

could you please add the PR to the Changelog?

@MrNightSky
Copy link
Contributor Author

Ok
I can add PR to Changelog. But I do it first time. Where to add this info? Like this?

[YY.MM.R] - YYYY-MM-DD
Added
example how to add extra params to DataLoader in config files
Changed
layerwise to correct key value layerwise_params in config files
Removed
Fixed
customizing what happens in train() notebook (#1203)

@Scitator
Copy link
Member

Scitator commented May 14, 2021 •
edited

fixed :)
you have fixed the docs, I think, just to notice the change

@Scitator Scitator merged commit c2368c9 into catalyst-team:master May 16, 2021
@MrNightSky
Copy link
Contributor Author

Done, but by another PR #1216

elephantmipt pushed a commit that referenced this pull request May 30, 2021
@MrNightSky @elephantmipt
Change layerwise to layerwise_params (#1210)
0af8667 
* 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
Scitator added a commit that referenced this pull request May 31, 2021
@elephantmipt @Scitator
@MrNightSky @ditwoo @bagxi @Dokholyan @riohib @jakubczakon @HubertJaworski @Herudaio
Add Registry to batch transform callback (#1209)
ac3887d 
* 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>
Scitator added a commit that referenced this pull request May 31, 2021
@Scitator @elephantmipt
@MrNightSky @ditwoo @bagxi @Dokholyan @riohib @jakubczakon @HubertJaworski @Herudaio @y-ksenia
Develop - version update (#1225)
403291c 
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ditwoo ditwoo ditwoo approved these changes

@Scitator Scitator Scitator approved these changes

@bagxi bagxi Awaiting requested review from bagxi bagxi is a code owner

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@MrNightSky @Scitator @ditwoo