-
Notifications
You must be signed in to change notification settings - Fork 72
[WIP] Add support for dynamic episode lengths #407
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,7 @@ namespace madrona_gpudrive | |
| m.attr("kMaxAgentCount") = consts::kMaxAgentCount; | ||
| m.attr("kMaxRoadEntityCount") = consts::kMaxRoadEntityCount; | ||
| m.attr("kMaxAgentMapObservationsCount") = consts::kMaxAgentMapObservationsCount; | ||
| m.attr("episodeLen") = consts::episodeLen; | ||
| m.attr("maxEpisodeLen") = consts::maxEpisodeLength; | ||
| m.attr("numLidarSamples") = consts::numLidarSamples; | ||
| m.attr("vehicleScale") = consts::vehicleLengthScale; | ||
|
|
||
|
|
@@ -146,7 +146,8 @@ namespace madrona_gpudrive | |
| }) | ||
| .def("deleted_agents_tensor", &Manager::deletedAgentsTensor) | ||
| .def("map_name_tensor", &Manager::mapNameTensor) | ||
| .def("scenario_id_tensor", &Manager::scenarioIdTensor); | ||
| .def("scenario_id_tensor", &Manager::scenarioIdTensor) | ||
| .def("episode_length_tensor", &Manager::episodeLengthTensor); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have the steps remaining tensor, is the only diff that this one is counting up? |
||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,7 @@ inline constexpr float rewardPerDist = 0.05f; | |
| inline constexpr float slackReward = -0.005f; | ||
|
|
||
| // Steps per episode | ||
| inline constexpr int32_t episodeLen = 91; | ||
| inline constexpr int32_t maxEpisodeLength = 200; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should stick to Also, let's keep the default value |
||
|
|
||
| // Number of lidar samples, arranged in circle around agent | ||
| inline constexpr madrona::CountT numLidarSamples = 50; | ||
|
|
@@ -57,7 +57,7 @@ inline constexpr madrona::CountT numPhysicsSubsteps = 0.f; | |
| inline constexpr float zDimensionScale = 1; | ||
| 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. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
|
||
| inline constexpr madrona::CountT kMaxRoadGeometryLength = 1810; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ namespace madrona_gpudrive | |
| // Constants computed from train files. | ||
| constexpr size_t MAX_OBJECTS = 515; | ||
| constexpr size_t MAX_ROADS = 956; | ||
| constexpr size_t MAX_POSITIONS = 91; | ||
| constexpr size_t MAX_POSITIONS = consts::maxEpisodeLength; | ||
| constexpr size_t MAX_GEOMETRY = 1746; | ||
|
|
||
| // Cannot use Madrona::math::Vector2 because it is not a POD type. | ||
|
|
@@ -124,6 +124,7 @@ namespace madrona_gpudrive | |
| bool disableClassicalObs = false; | ||
| DynamicsModel dynamicsModel = DynamicsModel::Classic; | ||
| bool readFromTracksToPredict = false; // Default: false - for womd_tracks_to_predict initialization mode | ||
| uint32_t episode_length = 91; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seeming inconsistency here: episode_length = 91 here and max episode length is 200? |
||
| }; | ||
|
|
||
| struct WorldInit | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,9 +18,10 @@ namespace madrona_gpudrive | |
| { | ||
| obj.mean = {0,0}; | ||
| uint32_t i = 0; | ||
| int numPositions = j.at("position").size(); | ||
| for (const auto &pos : j.at("position")) | ||
| { | ||
| if (i < MAX_POSITIONS) | ||
| if (i < MAX_POSITIONS && i < numPositions) | ||
| { | ||
| from_json(pos, obj.position[i]); | ||
| obj.mean.x += (obj.position[i].x - obj.mean.x)/(i+1); | ||
|
|
@@ -41,7 +42,7 @@ namespace madrona_gpudrive | |
| i = 0; | ||
| for (const auto &h : j.at("heading")) | ||
| { | ||
| if (i < MAX_POSITIONS) | ||
| if (i < MAX_POSITIONS && i < numPositions) | ||
| { | ||
| h.get_to(obj.heading[i]); | ||
| ++i; | ||
|
|
@@ -56,7 +57,7 @@ namespace madrona_gpudrive | |
| i = 0; | ||
| for (const auto &v : j.at("velocity")) | ||
| { | ||
| if (i < MAX_POSITIONS) | ||
| if (i < MAX_POSITIONS && i < numPositions) | ||
| { | ||
| from_json(v, obj.velocity[i]); | ||
| ++i; | ||
|
|
@@ -71,7 +72,7 @@ namespace madrona_gpudrive | |
| i = 0; | ||
| for (const auto &v : j.at("valid")) | ||
| { | ||
| if (i < MAX_POSITIONS) | ||
| if (i < MAX_POSITIONS && i < numPositions) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this block of code do? |
||
| { | ||
| v.get_to(obj.valid[i]); | ||
| ++i; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,7 @@ enum class ExportID : uint32_t { | |
| DeletedAgents, | ||
| MapName, | ||
| ScenarioId, | ||
| EpisodeLength, | ||
| NumExports | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -90,6 +90,11 @@ namespace madrona_gpudrive | |
| int32_t reset; | ||
| }; | ||
|
|
||
| struct EpisodeLength | ||
| { | ||
| int32_t episode_length; | ||
| }; | ||
|
|
||
| struct ResetMap { | ||
| int32_t reset; | ||
| }; | ||
|
|
@@ -337,15 +342,15 @@ namespace madrona_gpudrive | |
|
|
||
| struct Trajectory | ||
| { | ||
| madrona::math::Vector2 positions[consts::kTrajectoryLength]; | ||
| madrona::math::Vector2 velocities[consts::kTrajectoryLength]; | ||
| float headings[consts::kTrajectoryLength]; | ||
| float valids[consts::kTrajectoryLength]; | ||
| Action inverseActions[consts::kTrajectoryLength]; | ||
| madrona::math::Vector2 positions[consts::maxEpisodeLength]; | ||
| madrona::math::Vector2 velocities[consts::maxEpisodeLength]; | ||
| float headings[consts::maxEpisodeLength]; | ||
| float valids[consts::maxEpisodeLength]; | ||
| Action inverseActions[consts::maxEpisodeLength]; | ||
|
|
||
| static inline void zero(Trajectory& traj) | ||
| { | ||
| for (int i = 0; i < consts::kTrajectoryLength; i++) | ||
| for (int i = 0; i < consts::maxEpisodeLength; i++) | ||
| { | ||
| traj.positions[i] = {0, 0}; | ||
| traj.velocities[i] = {0, 0}; | ||
|
|
@@ -356,7 +361,7 @@ namespace madrona_gpudrive | |
| } | ||
| }; | ||
|
|
||
| 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; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks incorrect. Even if you increase the episode length, the data shape will always remain the same |
||
|
|
||
| static_assert(sizeof(Trajectory) == sizeof(float) * TrajectoryExportSize); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we stick to the original naming of episodeLen? The fact that an agent can be terminated before the episodeLen seems implied