Issue #213: Project invalid trajectories inside walkable area #506
Issue #213: Project invalid trajectories inside walkable area #506awestphal1 wants to merge 23 commits intoPedestrianDynamics:mainfrom
Conversation
…correct adjustments
schroedtert
left a comment
schroedtert
left a comment
There was a problem hiding this comment.
This is just the start to get the unit tests running again. Took a brief look at the pre-commit checks, some of them fail due to wrong format of the docstrings, which should be easily fixable. If you need help setting up the pre-commit checks locally let me know. I will have a more in-depth review in the next days.
There was a problem hiding this comment.
Can you move the tests into a preprocessing directory as you did for the implementation, e.g., tests/unit_tests/preprocessing/test_trajectory_projector.py?
schroedtert
left a comment
schroedtert
left a comment
There was a problem hiding this comment.
I finally had the time for a more in-depth review, but I skipped the math part, maybe someone else will have a look at that.
There are some general remarks, which we may have to discuss with the others. Personally I would prefer to get the functionality quickly into the main branch and do optimizations then. Otherwise it will take too long to get it included. What do you think.
- You transfer the shapely data structures into float values. Would't it be easier to just keep them and pass them to the functions?
- When dealing with DataFrames, Numpy and Shapely there is usually a way to avoid looping over everything and performing an action on an individual row/data point/geometry. My feeling tells me, that is also possible here, but I would also need a deeper dive into the code test it.
I really like that you already added some unit tests! This gives me a lot of confidence that this works as expected. And as an additional advantage it make optimization and refactoring way easier!
If you don't understand one of the comments or disagree, don't hesitate to contact me (RC or answer to the comment). The comments marked as optional you can resolve yourself if currently too much effort.
| a is equal to back_distance \n | ||
| b is equal to start_distance \n | ||
| c is equal to end_distance \n | ||
|
|
There was a problem hiding this comment.
This is highly optional!
If you have it already or it is not too difficult to create, a picture/plot showing the projection and the corresponding parameters would be great.
There was a problem hiding this comment.
I have made a plot to visualize the parameters, but I don’t know where to put it yet. I attached it here.
plot_different_parameters.pdf
I also changed the paramters start_-/end_distance to min_-/max_distance, because it seems easier to understand their purpose this way.
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Added a new preprocessing directory containing the trajectory_projector file.
The function correct_invalid_trajectories identifies every invalid point of a trajectory and moves it outside the geometry.
The direction and the new distance between the point and the wall are computed using trigonometric methods and linear interpolation. Points that lie deeper inside a wall are shifted a shorter distance outward, while points that are already close to the boundary are moved farther.
Closes #213