Improved trajectory_projector calculations through vectorization#542
Improved trajectory_projector calculations through vectorization#542awestphal1 wants to merge 44 commits into
Conversation
…correct adjustments
There was a problem hiding this comment.
Overall looks great, have only looked at the latest three commits containing the changes for the vectorization improvement.
It would be great if you could move the shapely normalization to the constructor of the WalkableArea (#521 and see comment). And if you could take a look at the missed lines in the code coverage and construct test cases that would cover them.
There was a problem hiding this comment.
The notebook is currently not shown on the webpage (https://pedpy--542.org.readthedocs.build/542/). If you need help setting that up, let me know
| "cell_type": "code", | ||
| "execution_count": null, | ||
| "id": "13", | ||
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "for i in range(len(invalid_person_ids)):\n", | ||
| " original_trajectory = trajectory_data.data[trajectory_data.data[\"id\"] == invalid_person_ids[i]]\n", | ||
| " trajectory_corrected = valid_trajectory.data[valid_trajectory.data[\"id\"] == invalid_person_ids[i]]\n", | ||
| " pedpy.plot_trajectories(\n", | ||
| " traj=pedpy.TrajectoryData(data=original_trajectory, frame_rate=trajectory_data.frame_rate),\n", | ||
| " walkable_area=walk_area,\n", | ||
| " hole_alpha=0,\n", | ||
| " ).set_aspect(\"equal\")\n", | ||
| " plt.xlabel(f\"personID {invalid_person_ids[i]} / original\")\n", | ||
| " plt.show()\n", | ||
| " traj_corr = pedpy.TrajectoryData(data=trajectory_corrected, frame_rate=trajectory_data.frame_rate)\n", | ||
| " pedpy.plot_trajectories(traj=traj_corr, walkable_area=walk_area, hole_alpha=0).set_aspect(\"equal\")\n", | ||
| " plt.xlabel(f\"personID {invalid_person_ids[i]} / corrected\")\n", | ||
| " plt.show()" |
There was a problem hiding this comment.
Can you add a hide-input tag to the cell? Then the code won't be shown directly on the webpage
| all_points_for_correcting = list(invalid_traj_lines.index) | ||
| # Those points very likely have to be corrected, but not necessarily | ||
| except GeometryError as p: | ||
| raise InputError("max_distance parameter is not valid") from p |
There was a problem hiding this comment.
why is it not valid? is it too small or too big? Could you add a hint to the user how they can avoid the error in the message?
| # type : wall_type - The type: 'wall' or 'obstacle' | ||
| # direction : int - 1 or -1 | ||
| # points : list - List with lists like [p1x, p1y, p2x, p2y], describing one edge of the wall | ||
| normalized_walk_area_polygon = shapely.normalize(walkable_area._polygon) |
There was a problem hiding this comment.
It would be better if this would be added directly to the constructor of the walkable area (see #521). Then we are sure that every operation on a WalkableArea is conducted on a normalized polygon.
| min_distance_obst: float, | ||
| max_distance_obst: float, | ||
| walkable_area: WalkableArea, | ||
| walkable_area: shapely.Polygon, |
There was a problem hiding this comment.
If the type of the parameter changes, I would like to also see the name changed to walkable_area_poly or similar. All other functions use the convention walkable_area == WalkableArea
| prev_x = data_trajectories.loc[all_points_for_correcting, "x"].to_numpy() | ||
| prev_y = data_trajectories.loc[all_points_for_correcting, "y"].to_numpy() |
There was a problem hiding this comment.
Please use column_identifier.X_COL and Y_COL here instead of plain x, y. That way whenever we would decide to change the name, it would be directly reflected here
| data_trajectories.loc[all_points_for_correcting, "x"] = new_x | ||
| data_trajectories.loc[all_points_for_correcting, "y"] = new_y |
There was a problem hiding this comment.
X_COL, Y_COL see above
| max_distance_obst: float, | ||
| walkable_area: WalkableArea, | ||
| ) -> tuple[float, float]: | ||
| walkable_area: shapely.Polygon, |
There was a problem hiding this comment.
See naming comment above
There was a problem hiding this comment.
Just a general hint:
I assume that the plots are created using matplotlib and some of the provided demo-data. In that case it would be great if you could add the creating of the plots to the readthedocs notebook. Then we could re-create the images easily when some behavior changes. If it is now too much work, that can be later also.
Note: the axis are not scaled equally, hence the geometry is distorted
Codecov Report❌ Patch coverage is ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Closes #518 #519 #520 #521
The trajectory_projector functions now have significantly better performance due to vectorization. Furthermore, I replaced large parts of the calculations with shapely and numpy methods. The functions now work directly on Shapely geometries, so a geo_data structure is no longer necessary.
The correct_invalid_trajectory function now also returns a list of all person IDs containing invalid trajectory points (similar to the outlier detection). This makes it possible to plot the modified trajectories before and after correction for easier comparison. I also added corresponding code to the preprocessing notebook.