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] Collection of small AIR output improvements #37571

Merged
merged 13 commits into from
Jul 20, 2023

Conversation

krfricke
Copy link
Contributor

@krfricke krfricke commented Jul 19, 2023

Why are these changes needed?

Includes #37531

This PR includes a number of improvements to Ray AIRs context-aware output engine

Related issue number

Closes #36911
Closes #36815
Closes #36818
Closes #36791

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 :(

Kai Fricke added 11 commits July 18, 2023 16:45
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Kai Fricke <kai@anyscale.com>
Kai Fricke added 2 commits July 19, 2023 15:52
add
Signed-off-by: Kai Fricke <kai@anyscale.com>
# Conflicts:
#	python/ray/tune/experimental/output.py
@@ -688,8 +688,6 @@
" # redirect logs to relative path instead of default ~/ray_results/\n",
" storage_path=\"my_Tune_logs\",\n",
" name=\"batch_tuning\",\n",
" # Set Ray Tune verbosity. Print summary table only with levels 2 or 3.\n",
Copy link
Member

Choose a reason for hiding this comment

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

Do we have a timeline to enable new output by default? Currently the RunConfig docstring is a little confusing.

verbose0, 1, 2, or 3. Verbosity mode. 0 = silent, 1 = only status updates, 
2 = status and brief results, 3 = status and detailed results. Defaults to 3. If the 
RAY_AIR_NEW_OUTPUT=1 environment variable is set, uses the new context-aware 
verbosity settings: 0 = silent, 1 = default, 2 = verbose.

Not needed for now, but would be great to have examples comparing outputs in verbosity level 0, 1, 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We aim to enable this per default in 2.7

@@ -85,6 +85,7 @@
"trial_id",
"experiment_tag",
"should_checkpoint",
"_report_on", # LIGHTNING_REPORT_STAGE_KEY
Copy link
Member

Choose a reason for hiding this comment

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

Actually this is created by us in LightningTrainer. Yes, we should put it into the blacklist.

shown_attributes["metrics"] = {
k: v for k, v in self.metrics.items() if k not in AUTO_RESULT_KEYS
k: v for k, v in self.metrics.items() if k not in exclude
Copy link
Member

Choose a reason for hiding this comment

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

So BLACKLISTED_KEYS will not be shown in Result and ResultGrid repr, but will still be printed by ProgressReporter on_trial_result, right? I feel like information like time_since_restore and iterations_since_restore are still important for progress tracking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this PR we're just removing it from the repr.

Currently the blacklisted keys are also not printed in the results table. Per default, only the most relevant metrics should be printed. If users want to show more metrics, they can customize this.

Maybe customization of this should be a blocker for enabling per default.

Copy link
Contributor

@matthewdeng matthewdeng left a comment

Choose a reason for hiding this comment

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

docs stamp

@krfricke krfricke merged commit bd1b4c9 into ray-project:master Jul 20, 2023
69 of 72 checks passed
@krfricke krfricke deleted the air/output/small branch July 20, 2023 16:02
NripeshN pushed a commit to NripeshN/ray that referenced this pull request Aug 15, 2023
…#37571)

This PR includes a number of improvements to Ray AIRs context-aware output engine

    Remove verbose=x from a number of examples (

[AIR output] Use verbose =1 by default in train or tune examples  ray-project#36911)
Use blocks to logically group outputs/control newlines (
[AIR output] Remove the blank line between reported results and checkpoint info.  ray-project#36815)
Exclude _report_on (
[AIR Output] Not sure if some metrics for lightning trainer are unnecessary ray-project#36818)
Extend blacklisted keys in result repr (
[AIR output] make the best result of tune output more readable ray-project#36791)

Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: NripeshN <nn2012@hw.ac.uk>
harborn pushed a commit to harborn/ray that referenced this pull request Aug 17, 2023
…#37571)

This PR includes a number of improvements to Ray AIRs context-aware output engine

    Remove verbose=x from a number of examples (

[AIR output] Use verbose =1 by default in train or tune examples  ray-project#36911)
Use blocks to logically group outputs/control newlines (
[AIR output] Remove the blank line between reported results and checkpoint info.  ray-project#36815)
Exclude _report_on (
[AIR Output] Not sure if some metrics for lightning trainer are unnecessary ray-project#36818)
Extend blacklisted keys in result repr (
[AIR output] make the best result of tune output more readable ray-project#36791)

Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: harborn <gangsheng.wu@intel.com>
harborn pushed a commit to harborn/ray that referenced this pull request Aug 17, 2023
…#37571)

This PR includes a number of improvements to Ray AIRs context-aware output engine

    Remove verbose=x from a number of examples (

[AIR output] Use verbose =1 by default in train or tune examples  ray-project#36911)
Use blocks to logically group outputs/control newlines (
[AIR output] Remove the blank line between reported results and checkpoint info.  ray-project#36815)
Exclude _report_on (
[AIR Output] Not sure if some metrics for lightning trainer are unnecessary ray-project#36818)
Extend blacklisted keys in result repr (
[AIR output] make the best result of tune output more readable ray-project#36791)

Signed-off-by: Kai Fricke <kai@anyscale.com>
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
…#37571)

This PR includes a number of improvements to Ray AIRs context-aware output engine

    Remove verbose=x from a number of examples (

[AIR output] Use verbose =1 by default in train or tune examples  ray-project#36911)
Use blocks to logically group outputs/control newlines (
[AIR output] Remove the blank line between reported results and checkpoint info.  ray-project#36815)
Exclude _report_on (
[AIR Output] Not sure if some metrics for lightning trainer are unnecessary ray-project#36818)
Extend blacklisted keys in result repr (
[AIR output] make the best result of tune output more readable ray-project#36791)

Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
vymao pushed a commit to vymao/ray that referenced this pull request Oct 11, 2023
…#37571)

This PR includes a number of improvements to Ray AIRs context-aware output engine

    Remove verbose=x from a number of examples (

[AIR output] Use verbose =1 by default in train or tune examples  ray-project#36911)
Use blocks to logically group outputs/control newlines (
[AIR output] Remove the blank line between reported results and checkpoint info.  ray-project#36815)
Exclude _report_on (
[AIR Output] Not sure if some metrics for lightning trainer are unnecessary ray-project#36818)
Extend blacklisted keys in result repr (
[AIR output] make the best result of tune output more readable ray-project#36791)

Signed-off-by: Kai Fricke <kai@anyscale.com>
Signed-off-by: Victor <vctr.y.m@example.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants