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

Separate computer_vision.py into its own test of enormous size. #35616

Merged
merged 5 commits into from
May 23, 2023

Conversation

gjoliver
Copy link
Member

@gjoliver gjoliver commented May 22, 2023

Why are these changes needed?

This test is too long.
Separate it into a different test.

Related issue number

Issue #35551
Issue #35692

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

@gjoliver gjoliver requested a review from a team as a code owner May 22, 2023 17:52
@gjoliver
Copy link
Member Author

looks good. the separated test passed:
Screen Shot 2023-05-22 at 2 47 06 PM

doc/BUILD Outdated
main = "source/ray-air/doc_code/computer_vision.py",
srcs = ["source/ray-air/doc_code/computer_vision.py"],
tags = ["exclusive", "team:ml"],
size = "enormous",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be large? Since it finishes in 566 < 900 secs.

Suggested change
size = "enormous",
size = "large",

Copy link
Member Author

Choose a reason for hiding this comment

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

apparently it runs faster once I separated it out.
it was timing out consistently at 900s.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we were supposed to avoid using the 'enormous' tag?

Copy link
Member Author

Choose a reason for hiding this comment

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

😕
so the options are splitting the actual script into 2.
or specify a different more specific limit?
do you have a preference?

@gjoliver gjoliver assigned richardliaw and unassigned matthewdeng May 22, 2023
Jun Gong added 4 commits May 23, 2023 12:16
Signed-off-by: Jun Gong <jungong@anyscale.com>
fix
Signed-off-by: Jun Gong <jungong@anyscale.com>
Signed-off-by: Jun Gong <jungong@anyscale.com>
Signed-off-by: Jun Gong <jungong@anyscale.com>
@gjoliver gjoliver force-pushed the computer_vision_test branch from 31c5f67 to 604cbce Compare May 23, 2023 19:17
Signed-off-by: Jun Gong <jungong@anyscale.com>
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.

approve as hotfix to unblock release.

@gjoliver gjoliver merged commit e15758e into ray-project:master May 23, 2023
gjoliver pushed a commit to gjoliver/ray that referenced this pull request May 24, 2023
…size. (ray-project#35616)

Signed-off-by: Jun Gong <jungong@anyscale.com>
ArturNiederfahrenhorst pushed a commit that referenced this pull request May 24, 2023
…size. (#35616) (#35693)

Signed-off-by: Jun Gong <jungong@anyscale.com>
scv119 pushed a commit to scv119/ray that referenced this pull request Jun 16, 2023
…size. (ray-project#35616)

Signed-off-by: Jun Gong <jungong@anyscale.com>
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
…size. (ray-project#35616)

Signed-off-by: Jun Gong <jungong@anyscale.com>
Signed-off-by: e428265 <arvind.chandramouli@lmco.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
Development

Successfully merging this pull request may close these issues.

3 participants