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

Speed up rllib test #1767

Merged
merged 1 commit into from
Dec 6, 2022
Merged

Speed up rllib test #1767

merged 1 commit into from
Dec 6, 2022

Conversation

Gamenot
Copy link
Collaborator

@Gamenot Gamenot commented Dec 6, 2022

Shorten the time of the test_rllib_hiway_env test.

@saulfield
Copy link
Contributor

What is the speed improvement for this? I think this test took around 30 mins on my machine originally.

@qianyi-sun
Copy link
Contributor

Not sure if this is reasonable, how about we create a even simpler scenario or duplicate an existing scenario just for the test and maybe gitignore it to prevent possible accidentally removal of it? I just have a feeling that in the future somehow we might forget that we have set this scenario for the test and delete it for some reason..

@Gamenot
Copy link
Collaborator Author

Gamenot commented Dec 6, 2022

Not sure if this is reasonable, how about we create a even simpler scenario or duplicate an existing scenario just for the test and maybe gitignore it to prevent possible accidentally removal of it? I just have a feeling that in the future somehow we might forget that we have set this scenario for the test and delete it for some reason..

I do not think that is possible since it is linked to the CI.

@Gamenot
Copy link
Collaborator Author

Gamenot commented Dec 6, 2022

What is the speed improvement for this? I think this test took around 30 mins on my machine originally.

I had the test take 3 minutes with this version. The original took 7:26. It might not be enough but it is a definite improvement.

@Gamenot Gamenot merged commit 9904e94 into develop Dec 6, 2022
@Gamenot Gamenot deleted the tucker/bugfix-speed_up_rllib_test branch December 6, 2022 21:46
@Gamenot
Copy link
Collaborator Author

Gamenot commented Dec 6, 2022

@saulfield I think the next step after may be to limit the number of CPUs. We could specifically set the number of CPUs to 1 or 2 and ensure that it is not hogging cores during test or we could use an environment variable in the test to limit the cores to be used in make test.

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