Fixes in unstructured constellation polygon creation#87
Conversation
fpasch
force-pushed
the
fix_and_cleanup_unstructured
branch
4 times, most recently
from
df8df1d to
1a7df03
Compare
berndgassmann
left a comment
berndgassmann
left a comment
There was a problem hiding this comment.
Reviewed 30 of 30 files at r1.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @fpasch)
ad_rss/impl/src/unstructured/TrajectoryCommon.cpp, line 72 at r1 (raw file):
int idx = 0; for (auto it = step.left.cbegin(); (it != step.left.cend()) && result; ++it)
result not actually used in the loop
ad_rss/impl/src/unstructured/TrajectoryCommon.cpp, line 113 at r1 (raw file):
boost::geometry::convex_hull(frontPtsLeft, hullLeft); boost::geometry::convex_hull(frontPtsRight, hullRight); result = combinePolygon(hullLeft, hullRight, polygon);
this is the first time result is used, so maybe move the declaration here and clean the code above
ad_rss/impl/src/unstructured/TrajectoryCommon.cpp, line 175 at r1 (raw file):
//------------- // front
where exactly is the difference between the computation between back and front? Looks like I am blind, but somehow I don't spot the difference... shouldn't the results be identical?
ad_rss/impl/src/unstructured/TrajectoryPedestrian.cpp, line 277 at r1 (raw file):
} auto previousRight = std::move(continueForwardFront.right); continueForwardFront.right = right;
This is an more efficient version of the below one, correct?
continueForwardFront.right.insert(continueForwardFront.right.begin(),right.begin(), right.end())
ad_rss/impl/src/unstructured/TrajectoryVehicle.cpp, line 120 at r1 (raw file):
backSide.right.reserve(vehicleState.dynamics.unstructuredSettings.vehicleBackIntermediateYawRateChangeRatioSteps + 1); auto result = getResponseTimeTrajectoryPoints(vehicleState, vehicleState.dynamics.alphaLon.accelMax, ratioDiffBack, backSide);
are you sure you want to use accelMax here, not brakeMax? otherwise front and back are the same... and while response time one also is allowed to brake, brakeMax <= a <= accelMax
Rather copy/paste error while restructuring.
ad_rss/impl/src/unstructured/TrajectoryVehicle.cpp, line 227 at r1 (raw file):
{ currentTime = duration; timeStep = physics::Duration(
why not add this before currentTime = duration; line:
timeStep = duration - currentTime;
or did I understand it wrong what's happening here within the last step
fpasch
force-pushed
the
fix_and_cleanup_unstructured
branch
from
1a7df03 to
6fbe8ff
Compare
- Add several parameters to adjust polygon calculation - Rename parameters: vehicleFrontIntermediateRatioSteps -> vehicleFrontIntermediateYawRateChangeRatioSteps vehicleBackIntermediateRatioSteps -> vehicleBackIntermediateYawRateChangeRatioSteps - code refactoring (pedestrian and vehicle calculations now use common functions)
fpasch
force-pushed
the
fix_and_cleanup_unstructured
branch
from
6fbe8ff to
ebb69a4
Compare
fpasch
left a comment
fpasch
left a comment
There was a problem hiding this comment.
Reviewable status: 16 of 31 files reviewed, 6 unresolved discussions (waiting on @berndgassmann)
ad_rss/impl/src/unstructured/TrajectoryCommon.cpp, line 72 at r1 (raw file):
Previously, berndgassmann wrote…
result not actually used in the loop
Done.
ad_rss/impl/src/unstructured/TrajectoryCommon.cpp, line 113 at r1 (raw file):
Previously, berndgassmann wrote…
this is the first time result is used, so maybe move the declaration here and clean the code above
Done.
ad_rss/impl/src/unstructured/TrajectoryCommon.cpp, line 175 at r1 (raw file):
Previously, berndgassmann wrote…
where exactly is the difference between the computation between back and front? Looks like I am blind, but somehow I don't spot the difference... shouldn't the results be identical?
Done.
ad_rss/impl/src/unstructured/TrajectoryPedestrian.cpp, line 277 at r1 (raw file):
Previously, berndgassmann wrote…
This is an more efficient version of the below one, correct?
continueForwardFront.right.insert(continueForwardFront.right.begin(),right.begin(), right.end())
yes
ad_rss/impl/src/unstructured/TrajectoryVehicle.cpp, line 120 at r1 (raw file):
Previously, berndgassmann wrote…
are you sure you want to use accelMax here, not brakeMax? otherwise front and back are the same... and while response time one also is allowed to brake, brakeMax <= a <= accelMax
Rather copy/paste error while restructuring.
Done.
ad_rss/impl/src/unstructured/TrajectoryVehicle.cpp, line 227 at r1 (raw file):
Previously, berndgassmann wrote…
why not add this before currentTime = duration; line:
timeStep = duration - currentTime;
or did I understand it wrong what's happening here within the last step
Done.
berndgassmann
left a comment
berndgassmann
left a comment
There was a problem hiding this comment.
Reviewed 15 of 15 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
Codecov Report
@@ Coverage Diff @@
## master #87 +/- ##
==========================================
+ Coverage 61.55% 61.68% +0.13%
==========================================
Files 132 132
Lines 4120 4119 -1
Branches 1813 1803 -10
==========================================
+ Hits 2536 2541 +5
- Misses 582 595 +13
+ Partials 1002 983 -19
Continue to review full report at Codecov.
|
3 participants
vehicleFrontIntermediateRatioSteps -> vehicleFrontIntermediateYawRateChangeRatioSteps
vehicleBackIntermediateRatioSteps -> vehicleBackIntermediateYawRateChangeRatioSteps
This change is