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

[air][tune] Aim logger #32041

Merged
merged 77 commits into from
Mar 3, 2023
Merged

[air][tune] Aim logger #32041

merged 77 commits into from
Mar 3, 2023

Conversation

ju2ez
Copy link
Contributor

@ju2ez ju2ez commented Jan 30, 2023

Why are these changes needed?

The default tensorboardx logger, though very powerful has some downsides to it like poor scalability (slow with many experiments). With this PR I want to give the option for an alternative (powerful) logger in form of the aim logger.

Related issue number

Closes #30537

Fixes #30674

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • proper unit testing of the written aim metrics
    • Release tests

ju2ez and others added 19 commits January 30, 2023 02:17
@ju2ez
added a first working draft of the aim_logger alongside unit test
fd93d57 
Signed-off-by: Julian <julianhatzky@googlemail.com>
@ju2ez
adjusted the logger/__init__.py. Now it imports the AimCallback
7276a39 
Signed-off-by: Julian <julianhatzky@googlemail.com>
@tamohannes @ju2ez
[feat] Add docs
61d1b6e 
Signed-off-by: Julian <julianhatzky@googlemail.com>
@tamohannes @ju2ez
[fix] Sign-off
f71c41e 
Signed-off-by: Julian <julianhatzky@googlemail.com>
@ju2ez
param check is now using pop
1b0ba2a 
Signed-off-by: Julian <julianhatzky@googlemail.com>
@ju2ez
metrics are now checked before they are logged.
af613e5 
Signed-off-by: Julian <julianhatzky@googlemail.com>
@tamohannes @ju2ez
[feat] Add tests
20b7f7d 
Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>
Signed-off-by: Julian <julianhatzky@googlemail.com>
@ju2ez
Removed comment
63b646b 
Signed-off-by: Julian <JulianHatzky@googlemail.com>
@tamohannes @ju2ez
[feat] Add as_multirun option. Update the docstring
99ffd41 
Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>
Signed-off-by: Julian <julianhatzky@googlemail.com>
@tamohannes @ju2ez
[feat] Add docs
2533be5 
Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>
@ju2ez
Fixed the imports and added try except statement for third party libr…
2d80acd 
…ary (aim)

Signed-off-by: Julian <julianhatzky@googlemail.com>
Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>
@ju2ez @justinvyu
Update python/ray/tune/logger/aim.py
59e3538 
Co-authored-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Julian <JulianHatzky@googlemail.com>
Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>
@ju2ez
Added assertion for aim import.
feb9f8a 
Signed-off-by: Julian <julianhatzky@googlemail.com>
Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>
@ju2ez
Type hint for _create_run(..)
b4de23f 
Signed-off-by: Julian <julianhatzky@googlemail.com>
Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>
@ju2ez
Renamed log_hparams function.
62210e3 
Signed-off-by: Julian <julianhatzky@googlemail.com>
Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>
@ju2ez
Fixed function call.
080cf1f 
Signed-off-by: Julian <julianhatzky@googlemail.com>
Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>
@ju2ez
Adjusted docstring and added the possibility to give kwargs to aim.sd…
1c701c6 
…k.Run class.

Signed-off-by: Julian <julianhatzky@googlemail.com>
Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>
@tamohannes @ju2ez
[feat] Add docs
d860cb8 
Signed-off-by: Julian <julianhatzky@googlemail.com>
@ju2ez
Small refinements
7bacdb8 
Signed-off-by: Julian <julianhatzky@googlemail.com>
@ju2ez ju2ez mentioned this pull request Jan 30, 2023
Closed
7 tasks
richardliaw
richardliaw requested changes Feb 7, 2023
View reviewed changes
Copy link
Contributor

@richardliaw richardliaw left a comment

Choose a reason for hiding this comment

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

mostly looks good!

python/ray/tune/logger/__init__.py Outdated Show resolved Hide resolved
python/ray/tune/logger/aim.py Outdated Show resolved Hide resolved
@justinvyu justinvyu requested a review from richardliaw February 7, 2023 20:55
@ju2ez
Copy link
Contributor Author

ju2ez commented Feb 13, 2023

@justinvyu @richardliaw what's left here to do?

@justinvyu
Copy link
Contributor

@justinvyu @richardliaw what's left here to do?

Hey @ju2ez, we're currently waiting on a patch release on aim's side to fix its requirements.txt. Currently, aim fails on our CI due to the version of a certain dependency being incompatible and erroring out. Should be on track to merge it in this week!

@justinvyu
Merge branch 'master' of https://github.com/ray-project/ray into aiml…
f59a78c 
…ogger
@justinvyu
Pin aim to 3.16.1 w/ versioning patch fix for CI to pass
7a39431 
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
@justinvyu
Fix aim logger tests
04d0b1e 
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
@justinvyu
Merge branch 'master' of https://github.com/ray-project/ray into aiml…
b17eb00 
…ogger
@justinvyu
Update docs to follow new format
01a6541 
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
@justinvyu
Fix text in user guide
c2ecbd0 
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
@justinvyu justinvyu self-requested a review March 2, 2023 01:24
justinvyu
justinvyu approved these changes Mar 2, 2023
View reviewed changes
Copy link
Contributor

@justinvyu justinvyu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@richardliaw Should be good after tests pass. Could you also switch your review to approve after a last look?

@justinvyu justinvyu added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Mar 2, 2023
justinvyu added 5 commits March 2, 2023 11:04
@justinvyu
Don't import aim callback by default
6c94f37 
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
@justinvyu
Merge branch 'master' of https://github.com/ray-project/ray into aiml…
48b6341 
…ogger
@justinvyu
Fix module path in API ref
29d1caa 
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
@justinvyu
Fix tests + example imports
6b02036 
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
@justinvyu
Merge branch 'master' of https://github.com/ray-project/ray into aiml…
80be6f9 
…ogger
@richardliaw richardliaw merged commit 98cee18 into ray-project:master Mar 3, 2023
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request Mar 21, 2023
@ju2ez @ProjectsByJackHe
[air][tune] Aim logger (ray-project#32041)
55a1428 
Co-authored-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com>
Co-authored-by: tmynn <hovhannes.tamoyan@gmail.com>
Co-authored-by: Justin Yu <justinvyu@berkeley.edu>
Closes ray-project#30537
Fixes ray-project#30674

Signed-off-by: Jack He <jackhe2345@gmail.com>
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
@ju2ez @edoakes
[air][tune] Aim logger (ray-project#32041)
e6dbd82 
Co-authored-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com>
Co-authored-by: tmynn <hovhannes.tamoyan@gmail.com>
Co-authored-by: Justin Yu <justinvyu@berkeley.edu>
Closes ray-project#30537
Fixes ray-project#30674

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
peytondmurray pushed a commit to peytondmurray/ray that referenced this pull request Mar 22, 2023
@ju2ez @peytondmurray
[air][tune] Aim logger (ray-project#32041)
5947002 
Co-authored-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com>
Co-authored-by: tmynn <hovhannes.tamoyan@gmail.com>
Co-authored-by: Justin Yu <justinvyu@berkeley.edu>
Closes ray-project#30537
Fixes ray-project#30674
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
@ju2ez @elliottower
[air][tune] Aim logger (ray-project#32041)
44de093 
Co-authored-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com>
Co-authored-by: tmynn <hovhannes.tamoyan@gmail.com>
Co-authored-by: Justin Yu <justinvyu@berkeley.edu>
Closes ray-project#30537
Fixes ray-project#30674

Signed-off-by: elliottower <elliot@elliottower.com>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
@ju2ez @ProjectsByJackHe
[air][tune] Aim logger (ray-project#32041)
101de31 
Co-authored-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com>
Co-authored-by: tmynn <hovhannes.tamoyan@gmail.com>
Co-authored-by: Justin Yu <justinvyu@berkeley.edu>
Closes ray-project#30537
Fixes ray-project#30674

Signed-off-by: Jack He <jackhe2345@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@angelinalg angelinalg angelinalg left review comments

@justinvyu justinvyu justinvyu approved these changes

@richardliaw richardliaw richardliaw approved these changes

@krfricke krfricke Awaiting requested review from krfricke

@xwjiang2010 xwjiang2010 Awaiting requested review from xwjiang2010

@amogkam amogkam Awaiting requested review from amogkam

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

@Yard1 Yard1 Awaiting requested review from Yard1

@maxpumperla maxpumperla Awaiting requested review from maxpumperla

Assignees

@justinvyu justinvyu

Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Integration of aim metric tracker as an alternative to tensorboardx in ray tune
5 participants
@ju2ez @angelinalg @tamohannes @justinvyu @richardliaw