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

[Actor] [Code Quality] Add Unit Tests for Actors Sorting #34058

Merged
merged 62 commits into from
Apr 10, 2023

Conversation

chaowanggg
Copy link
Contributor

@chaowanggg chaowanggg commented Apr 4, 2023

Why are these changes needed?

Following with #33395 (comment), add a component test to improve the code quality

Related issue number

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

chaowanggg and others added 30 commits March 20, 2023 21:00
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Deletes the old IA.

Deleted all routes from old IA that were no longer used. (One remained: CmdResult)
Moved all the new IA routes from /new/<route> to /<route>
Deleted all components that were no longer used as a result of removing old IA routes
Deleted all parameters for "newIA" and removed the code paths where newIA is false
Deleted the dark theme from the the app (theme was only adjustable in the old IA)
I manually tested every single button to make sure links still worked. Our testing on the dashboard is not quite good enough yet (it's getting better) to trust the tests to catch all possible bugs here. The earlier we get this into nightly, the more manual testing from users we can get.

Signed-off-by: chaowang <[email protected]>
…project#31578)

Why are these changes needed?
Changes made to batch_tuning.ipynb and batch_forecasting.ipynb:

Update notebook texts, make steps clearer.
Tune outputs only showing SMOKE_TEST outputs.

---------

Signed-off-by: Christy Bergman <[email protected]>
Signed-off-by: Antoni Baum <[email protected]>
Co-authored-by: Antoni Baum <[email protected]>
Signed-off-by: chaowang <[email protected]>
70dbf41 pinned typeguard to make ax-platform work again on latest master, but a8a1ed0 updated ax-platform for python >= 3.8. The typeguard pin is incompatible with this pinned version. So we need to pin typeguard only for Python 3.7 to make the image builds for 3.8+ work again.

Signed-off-by: Kai Fricke <[email protected]>
Signed-off-by: chaowang <[email protected]>
…ject#33370)

The naming of different path-related components in Ray Tune is currently messy. For instance, `Experiment.local_dir` refers to main results directory, e.g. `~/ray_results`, while `Trial.local_dir` refers to the experiment directory, e.g. `~/ray_results/experiment`. The same is true for properties, where it's unclear if it refers to the object's sub directory or to its parent directory.

To disentangle this information, this PR introduces a new naming convention.

- All entities receiving a "parent path" receive it in a unambiguous naming scheme.
- For instance, `Experiment(storage_path)`, `TrialRunner(experiment_path)`, `Trial(experiment_path)`
- Outputs are also normalized. E.g. `Trial.remote_experiment_path` and `Trial.local_experiment_path`
- We keep existing arguments and properties for backwards compatibility

Signed-off-by: Kai Fricke <[email protected]>
Signed-off-by: chaowang <[email protected]>
…ct#33288)

Use SDK commands for all core tests.

It is because there was a big regression after migrating to V2 anyscale job runner.

Signed-off-by: chaowang <[email protected]>
dbczumar and others added 12 commits April 4, 2023 18:24
The current node option verification attempts to convert a string key to a dictionary when constructing an error message for blocked options, resulting in unclear / unintended exception.

Signed-off-by: dbczumar <[email protected]>
Signed-off-by: chaowang <[email protected]>
For long-living, memory leaking actors, it is more desirable to kill oldest task that is leaking the most. This avoid the situation where we constantly kill actor, which may lead to side effects where we generate a lot of log files, or trigger increased memory consumption in gcs / dashboard

Co-authored-by: Clarence Ng <[email protected]>
Signed-off-by: chaowang <[email protected]>
EBS is not necessary for many nodes test and it's 150GB by default. This reduce it to 30GB.
---------

Co-authored-by: Chen Shen <[email protected]>
Signed-off-by: chaowang <[email protected]>
CPP generate-bazel-project-template is failing during build, due to extra space in the BUILD.bazel file. This commit fixes that.

Signed-off-by: Soumitra Kumar <[email protected]>
Signed-off-by: chaowang <[email protected]>
This PR restricts the number of profile events to be sent and aggregates task events from the same task attempt on the worker side to reduce the data sent to GCS.
This PR also refactos the metrics tracking to reduce lock contention on the core worker.

Signed-off-by: chaowang <[email protected]>
…t#32936)

Co-authored-by: angelinalg <[email protected]>
Co-authored-by: Justin Yu <[email protected]>
Co-authored-by: Yunxuan Xiao <[email protected]>
Co-authored-by: Yunxuan Xiao <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
Signed-off-by: chaowang <[email protected]>
@chaowanggg chaowanggg changed the title Cw/component test actors [Actor] [Code Quality] Add Unit Tests for Actors Sorting Apr 4, 2023
@chaowanggg
Copy link
Contributor Author

@alanwguo @shawnpanda Please review

@alanwguo
Copy link
Contributor

alanwguo commented Apr 6, 2023

@chaowanggg for the "DCO" failure. You need to make sure to use -s for every git commit you do. Example git commit -s.

This makes it so that you "sign" every commit with your name at the bottom. This is required for the OSS ray repo

@rkooo567 rkooo567 merged commit 6db3f1f into ray-project:master Apr 10, 2023
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
…#34058)

Following with ray-project#33395 (comment), add a component test to improve the code quality

Signed-off-by: elliottower <[email protected]>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
…#34058)

Following with ray-project#33395 (comment), add a component test to improve the code quality

Signed-off-by: Jack He <[email protected]>
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.