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

Cherry pick pin gym dependency #23705

Merged
merged 6 commits into from
Apr 8, 2022
Merged

Cherry pick pin gym dependency #23705

merged 6 commits into from
Apr 8, 2022

Conversation

krfricke
Copy link
Contributor

@krfricke krfricke commented Apr 4, 2022 •
edited

Why are these changes needed?

Cherry-picking dependency pins for gym.

Related issue number

Closes #23530

Checks

  • 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
    • Release tests
    • This PR is not tested :(

avnishn and others added 5 commits April 4, 2022 15:03
@avnishn
[release tests] Pin gym everywhere (ray-project#23349)
02480e4
@avnishn
[rllib] Pin gym everywhere (ray-project#23384)
e9fcb9d 
This PR Pins gym in the app config.yaml's for rllib and tune so that release tests are no longer broken by the new gym version.
@avnishn
[RLlib] Pin Gym Everywhere and turn off gpu for recsim tests (ray-pro…
7ab2814 
…ject#23452)
@ddelange @krfricke
[ci] Clean up ray-ml requirements (ray-project#23325)
cb156d7 
In https://github.com/ray-project/ray/blob/ray-1.11.0/docker/ray-ml/Dockerfile, the order of pip install commands currently matters (potentially a lot). It would be good to run one big pip install command to avoid ending up with a broken env.

Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com>
@amogkam
Fix merge conflict
cc3b4aa
@krfricke
Copy link
Contributor Author

krfricke commented Apr 5, 2022

Seems there is a problem with requirements_train.txt - I'll look into this tomorrow morning

@architkulkarni
Copy link
Contributor

architkulkarni commented Apr 5, 2022 •
edited

CI privileged-test and post-wheel-test were failing on the 1.11.0 branch as well and the decision was made to ignore them, so we'll ignore them for 1.11.1 as well.

Cherry picks need TL approval, @richardliaw or @matthewdeng or @sven1977 could you please approve this PR? (Sorry if I missed someone)

@architkulkarni
Copy link
Contributor

//python/ray/workflow:tests/test_virtual_actor_2 is failing consistently on this PR, but it's passing on the last few commits of the 1.11.0 branch and the 1.11.1 branch. Could it be related to this PR? It recently started failing on master, but not sure how that could affect this PR. @iycheng any idea about this?

@architkulkarni
Copy link
Contributor

//python/ray/workflow:tests/test_virtual_actor_2 is failing consistently on this PR, but it's passing on the last few commits of the 1.11.0 branch and the 1.11.1 branch. Could it be related to this PR? It recently started failing on master, but not sure how that could affect this PR. @iycheng any idea about this?

@clarkzinzow can you help diagnose this? Is this safe to merge or do we need some kind of fix?

@clarkzinzow
Copy link
Contributor

clarkzinzow commented Apr 7, 2022 •
edited

@architkulkarni After looking at the tests and Workflows code, and comparing with the diff, I'm not sure how this PR could cause these tests to start timing out... These tests have also started being flakey in master, so it might be related to CI infra instead of the diff, although those failures don't all appear to be timeouts.

@iycheng Could you take a look at this? I don't think that I have enough context around Workflows to debug this in a timely manner.

@architkulkarni
Copy link
Contributor

I'm going to merge this so we can start the release tests. If we determine later (before the final release) that this PR caused the test failure, we can revert it, but I think it's really unlikely.

@architkulkarni architkulkarni merged commit 8a3e92b into ray-project:releases/1.11.1 Apr 8, 2022
@clarkzinzow
Copy link
Contributor

@architkulkarni If you're talking about the Workflows test_virtual_actor_2 tests, those are flaky on master as well, so I wouldn't worry about it.

@krfricke krfricke deleted the cherry-pick-gym branch September 22, 2023 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewdeng matthewdeng matthewdeng approved these changes

@richardliaw richardliaw richardliaw approved these changes

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

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

@architkulkarni architkulkarni Awaiting requested review from architkulkarni

Assignees

@matthewdeng matthewdeng

@richardliaw richardliaw

@architkulkarni architkulkarni

@sven1977 sven1977

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@krfricke @architkulkarni @clarkzinzow @matthewdeng @richardliaw @sven1977 @avnishn @ddelange @amogkam