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.

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

[air/output] Print experiment information at experiment start #34952

Merged
merged 19 commits into from
May 4, 2023

Conversation

krfricke
Copy link
Contributor

@krfricke krfricke commented May 2, 2023

Why are these changes needed?

Includes #34788

This PR prints experiment information at the start of the experiment.

@scottsun94 should this also happen when rich is enabled? (Currently disabled enabled without config table).

Example:

╭───────────────────────────────────────────────────────────────────────╮
│ Configuration for experiment     easy_objective_2023-05-02_16-10-37   │
├───────────────────────────────────────────────────────────────────────┤
│ Search algorithm                 BasicVariantGenerator                │
│ Scheduler                        FIFOScheduler                        │
│ Number of trials                 10                                   │
╰───────────────────────────────────────────────────────────────────────╯

View detailed results here: /Users/kai/ray_results/easy_objective_2023-05-02_16-10-37
To visualize your results with TensorBoard, run: `tensorboard --logdir /Users/kai/ray_results/easy_objective_2023-05-02_16-10-37`

Related issue number

Closes #34920
Closes #33154

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 added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • 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
    • Release tests
    • This PR is not tested :(

@krfricke krfricke changed the title Air/output/config on start @krfricke [air/output] Print experiment information at experiment start May 2, 2023
@krfricke krfricke changed the title @krfricke [air/output] Print experiment information at experiment start [air/output] Print experiment information at experiment start May 2, 2023
Kai Fricke added 2 commits May 2, 2023 16:25
Signed-off-by: Kai Fricke <[email protected]>
Signed-off-by: Kai Fricke <[email protected]>
@scottsun94
Copy link
Contributor

scottsun94 commented May 2, 2023

Should this also happen when rich is enabled?

I feel like that this is orthogonal to Rich and trial table. WDYT?

# Conflicts:
#	python/ray/tune/experimental/output.py
@krfricke krfricke marked this pull request as ready for review May 2, 2023 19:47
@krfricke
Copy link
Contributor Author

krfricke commented May 2, 2023

Should this also happen when rich is enabled?

I feel like that this is orthogonal to Rich and trial table. WDYT?

Yep SGTM - currently it's always being printed and I think that's how it should be.

@scottsun94
Copy link
Contributor

Should this also happen when rich is enabled?

I feel like that this is orthogonal to Rich and trial table. WDYT?

Yep SGTM - currently it's always being printed and I think that's how it should be.

SGTM. Thanks!

@krfricke krfricke added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label May 3, 2023
@krfricke krfricke requested a review from gjoliver May 3, 2023 15:05
Copy link
Member

@gjoliver gjoliver left a comment

Choose a reason for hiding this comment

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

cool cool!
also a couple of nits.

@@ -463,6 +464,31 @@ def __init__(self, verbosity: AirVerbosity):
self._start_time = time.time()
self._last_heartbeat_time = 0

def experiment_started(
Copy link
Member

Choose a reason for hiding this comment

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

hey we should mark all the public APIs with @DeveloperAPI or something.
so it's very clear what functions one can override to completely customize the terminal output.
wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it's currently experimental we don't need annotations just now (and should probably avoid them so we can continue to quickly iterate).

Before committing to the APIs, should we have a separate discussion on how and what users should be able to configure or overwrite? I don't think many people are going to write their own reporters tbh. It's more likely that they'll use configuration options to control the output, and the reporter interface is for us to implement new handlers/features if needed.

python/ray/tune/experimental/output.py Outdated Show resolved Hide resolved
@krfricke krfricke merged commit 581967f into ray-project:master May 4, 2023
@krfricke krfricke deleted the air/output/config-on-start branch May 4, 2023 10:17
architkulkarni pushed a commit to architkulkarni/ray that referenced this pull request May 16, 2023
…oject#34952)

Includes ray-project#34788

This PR prints experiment information at the start of the experiment.

Signed-off-by: Kai Fricke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
3 participants