[WIP] Add support for dynamic episode lengths #407
Conversation
daphne-cornelisse
left a comment
daphne-cornelisse
left a comment
There was a problem hiding this comment.
Overall, a couple of questions:
- What is the function of the original steps_remaining tensor and the added episode length tensor?
- Can you briefly explain the logic behind your current implementation?
Left several comments in the code, too.
Note that changing names such as episodeLen also requires changes in the Python side before merging. Happy to help with this part down the line
| m.attr("kMaxRoadEntityCount") = consts::kMaxRoadEntityCount; | ||
| m.attr("kMaxAgentMapObservationsCount") = consts::kMaxAgentMapObservationsCount; | ||
| m.attr("episodeLen") = consts::episodeLen; | ||
| m.attr("maxEpisodeLen") = consts::maxEpisodeLength; |
There was a problem hiding this comment.
Can't we stick to the original naming of episodeLen? The fact that an agent can be terminated before the episodeLen seems implied
| .def("map_name_tensor", &Manager::mapNameTensor) | ||
| .def("scenario_id_tensor", &Manager::scenarioIdTensor); | ||
| .def("scenario_id_tensor", &Manager::scenarioIdTensor) | ||
| .def("episode_length_tensor", &Manager::episodeLengthTensor); |
There was a problem hiding this comment.
We already have the steps remaining tensor, is the only diff that this one is counting up?
|
|
||
| // Steps per episode | ||
| inline constexpr int32_t episodeLen = 91; | ||
| inline constexpr int32_t maxEpisodeLength = 200; |
There was a problem hiding this comment.
I think we should stick to episodeLen
Also, let's keep the default value
| inline constexpr float xDimensionScaleRoadSegment = 1; | ||
|
|
||
| inline constexpr madrona::CountT kTrajectoryLength = 91; // Nocturne has 90 timesteps per episode. making it 91 as a buffer. | ||
| // inline constexpr madrona::CountT kTrajectoryLength = 91; // Nocturne has 90 timesteps per episode. making it 91 as a buffer. |
There was a problem hiding this comment.
Even if we support variable episode lengths, the log replay trajectories in the scenes will remain of shape 91.
Why is this deleted? There is an important difference between the length of the episode and the length of the data in the simulator
| bool disableClassicalObs = false; | ||
| DynamicsModel dynamicsModel = DynamicsModel::Classic; | ||
| bool readFromTracksToPredict = false; // Default: false - for womd_tracks_to_predict initialization mode | ||
| uint32_t episode_length = 91; |
There was a problem hiding this comment.
Seeming inconsistency here: episode_length = 91 here and max episode length is 200?
| for (const auto &v : j.at("valid")) | ||
| { | ||
| if (i < MAX_POSITIONS) | ||
| if (i < MAX_POSITIONS && i < numPositions) |
There was a problem hiding this comment.
What does this block of code do?
| }; | ||
|
|
||
| const size_t TrajectoryExportSize = 2 * 2 * consts::kTrajectoryLength + 2 * consts::kTrajectoryLength + ActionExportSize * consts::kTrajectoryLength; | ||
| const size_t TrajectoryExportSize = 2 * 2 * consts::maxEpisodeLength + 2 * consts::maxEpisodeLength + ActionExportSize * consts::maxEpisodeLength; |
There was a problem hiding this comment.
This looks incorrect. Even if you increase the episode length, the data shape will always remain the same
No description provided.