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

Eliminate waymo_open_dataset dependency #1905

Merged
merged 11 commits into from
Mar 17, 2023
Merged

Eliminate waymo_open_dataset dependency #1905

merged 11 commits into from
Mar 17, 2023

Conversation

saulfield
Copy link
Contributor

Fixes #1880

@Gamenot
Copy link
Collaborator

Gamenot commented Mar 14, 2023

I am cautious about naming the package the same as the original. It might cause a package conflict.

@saulfield
Copy link
Contributor Author

I am cautious about naming the package the same as the original. It might cause a package conflict.

I agree in general, but in this case I think it's ok. It makes it easier since we don't have to modify any of the protos or generated files to make the imports work, and since the waymo package is incompatible with smarts anyway there isn't a risk of them being installed in the same venv.

@Gamenot
Copy link
Collaborator

Gamenot commented Mar 14, 2023

OK.

@Adaickalavan
Copy link
Member

Some thoughts on presenting SMARTS support for external datasets. We could address this in a separate PR too.

Consider updating how to use SMARTS for Waymo in ReadTheDocs::Ecosystem::Waymo.

In ReadTheDocs::Example::Naturalistic section we could provide intructions
(e.g., scl run --envision examples/<script_name>.py scenarios/<scenario_name>)
to run a simple ego agent (e.g., AgentType.Laner, etc) on a Waymo map with traffic (e.g., dataset="training_20s.tfrecord-00022-of-01000" and scenario_id="62662fab4d63c247"). We could then post a gif of the replayed Waymo scenario showing the expected outcome. To reproduce the result, we could instruct the users to download and edit the dataset path in the provided script prior to executing the instructions.

We could move readmes such as (i)SMARTS/scenarios/waymo/README.md, and (ii) SMARTS/waymo_open_dataset/README.md, to ReadTheDocs::Ecosystem::Waymo .

@saulfield
Copy link
Contributor Author

I moved the waymo_open_dataset module to smarts/waymo because there were some issues with finding the module path from within smarts.

I also updated the Waymo section of the docs, and deleted the README in the waymo example scenario.

@saulfield saulfield merged commit 3d94dd0 into master Mar 17, 2023
@saulfield saulfield deleted the saul/waymo-module branch March 17, 2023 17:35
@Gamenot Gamenot mentioned this pull request May 23, 2023
17 tasks
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.

Waymo Dataset Incompatibility
4 participants