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

CI test for RL examples: Drive and VehicleFollowing #2062

Merged
merged 7 commits into from
Jun 13, 2023

Conversation

Adaickalavan
Copy link
Member

  • Added CI test for RL Drive and VehicleFollowing examples, which tests the RL training process.
  • Removed make build-all-scenarios command and only build necessary scenarios in CI to reduce test time.
  • Note: The CI test for this PR will pass after Update RL examples: Drive and VehicleFollowing #2061 is merged.

@@ -6,7 +6,7 @@ env:
venv_dir: .venv

jobs:
base-tests-linux:
base-tests:
Copy link
Collaborator

@Gamenot Gamenot Jun 12, 2023

Choose a reason for hiding this comment

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

I am not against it but is there any reason to change the name of the test?

Copy link
Member Author

Choose a reason for hiding this comment

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

With the addition of more jobs in the same workflow, following the original naming style, each of the job would have been suffixed with -linux. The suffix appeared a little repetitive, given the workflow's name of SMARTS CI Base Tests Linux which indicates it is for Linux. In short, the change was for aesthetics. This is how the test results look like now.

Tests

CHANGELOG.md Outdated Show resolved Hide resolved
-k 'not test_long_determinism'

examples-rl:
Copy link
Collaborator

@Gamenot Gamenot Jun 12, 2023

Choose a reason for hiding this comment

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

How long do these two tests take to run? We would want the total to be ~2 mins or less.

Otherwise, we will need to put it on some other schedule than always run.

Copy link
Member Author

Choose a reason for hiding this comment

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

  • Currently the examples-rl test takes ~20 minutes
  • By reducing the number of routes generated in the scenarios, the examples-rl test time may be reduced to ~12 minutes
  • Previously the base-tests test took ~53 minutes to run when it attempted to build all scenarios.
  • By only building the necessary scenarios, the base-tests now takes ~6 minutes.
  • For a quick benefit, we may make another PR consisting of only building the necessary scenarios which will hasten the base-tests, while we figure how to best do CI for examples-rl test.

.github/workflows/ci-base-tests-linux.yml Outdated Show resolved Hide resolved
@Gamenot
Copy link
Collaborator

Gamenot commented Jun 13, 2023

The test times appear to be ~18 minutes and 7 minutes. I think we should find a way to run these less frequently, maybe only against certain commit messages and master.

@Adaickalavan Adaickalavan merged commit befa515 into master Jun 13, 2023
@Adaickalavan Adaickalavan deleted the adai/ci-rl-example branch June 13, 2023 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants