Skip to content

Turning angles with continuous lane changes and lcSigma #11882

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

Closed
wants to merge 3 commits into from

Conversation

Domsall
Copy link
Contributor

@Domsall Domsall commented Oct 24, 2022

This pull request belongs to #8529.

The "new" Code part is the geometric calculation in calcAngleOffset and works as described in the last image.
As it uses speedLat, I needed to make sure that it always gets calculated.

I did not want to update all the tests that now change because of the fcd-output, so please do some tests before integrating.

@namdre namdre self-assigned this Oct 24, 2022
@Domsall
Copy link
Contributor Author

Domsall commented Oct 26, 2022

I just recognized that my last commit

if (p1 == Position::INVALID && vehicle->getLane()->getShape().length2D() == 0. && vehicle->getLane()->isInternal()) {
    // workaround: extrapolate the preceding lane shape
    MSLane* predecessorLane = vehicle->getLane()->getCanonicalPredecessorLane();
    p1 = predecessorLane->geometryPositionAtOffset(predecessorLane->getLength() + vehicle->myState.myPos, lefthandSign * posLat);
}

does something similar, that is already done a few lines later (was implemented a long time ago: 33d7c91):

if (vehicle->getLane()->getShape().length2D() == 0) {
    if (vehicle->getFurtherLanes().size() == 0) {
        laneAngle = vehicle->getAngle();
    } else {
        laneAngle = vehicle->getFurtherLanes().front()->getShape().rotationAtOffset(-NUMERICAL_EPS);
    }
}

At this point, I am not sure which implementation works better.

@namdre
Copy link
Contributor

namdre commented Nov 1, 2022

Was your last commit added in response to a test case where my old code didn't work? If so, attach the test case and I'll check.

@namdre
Copy link
Contributor

namdre commented Nov 1, 2022

Something else: so far the relationship between lateral position and lateral speed was the same as for longitudinal position and longitudinal speed (for the default non-ballistic update): deltaPos = speed * stepLength.
This means, if the position changes from one step to the next, the speed isn't 0.
With your change in MSLaneChanger::continueChange of lcm.setSpeedLat(0.0); this no longer holds.

@namdre
Copy link
Contributor

namdre commented Nov 1, 2022

I looked at the angle during continuous lane change (lc_model/continuous_lanechange/basic)
Here is the old plot:
Bildschirmfoto von 2022-11-01 10-23-46

Here is the new plot:
Bildschirmfoto von 2022-11-01 10-24-07

I find the new shape surprising.

For the sublane model I looked at test sublane_model/strategic/overtake_stopped_on_the_left

old:
Bildschirmfoto von 2022-11-01 10-29-57

new:

Bildschirmfoto von 2022-11-01 10-30-54

again there is a small plateau before the maneuver ends though it's less pronounced. Any idea whats happening there?

Third question: what tests are good to look at to see the improvement of the new code? Or do we need more tests?

@namdre
Copy link
Contributor

namdre commented Nov 1, 2022

FYI: I don't think i'll manage to merge this into the 1.15.0 release but would like to merge post-release

@Domsall
Copy link
Contributor Author

Domsall commented Nov 4, 2022

Thanks for your feedback and sorry for the late response, I took a few days off.

Was your last commit added in response to a test case where my old code didn't work? If so, attach the test case and I'll check.

It corresponds to issue #5879, which has the test sumo\bugs\5xxx\ticket5879. You can change --lanechange.duration 2.5 to --lateral-resolution 0.8 and should see a difference, if the edges "begin" and "end" have some curvature.
After looking into it a bit more, I would delete 33d7c91 (old angle calculation from one point) and keep the part from the last commit (angle calculation from two points).

  1. How can I further help with this?

Something else: so far the relationship between lateral position and lateral speed was the same as for longitudinal position and longitudinal speed (for the default non-ballistic update): deltaPos = speed * stepLength.
This means, if the position changes from one step to the next, the speed isn't 0.
With your change in MSLaneChanger::continueChange of lcm.setSpeedLat(0.0); this no longer holds.

You are absolutely right. I thought the same when changing this. The issue I am having is: When the lane change ends, SpeedLat and the Angle get calculated one last time in the LC-part of the Code. In the next step, SpeedLat is still "active" in the Angle calculation, because the Angle gets updated in the CF-part and SpeedLat gets updated after that in the LC-part of the Code. computeAngle() could be added to the LC-part. But then it would always be calculated twice (or we delete it from the CF-part).

  1. Do you have a good suggestion?

I looked at the angle during continuous lane change (lc_model/continuous_lanechange/basic)

I actually used that test as reference myself. The differences we see come from:

  • The lane change has 3 parts: Turning the steering wheel to a specific value, holding it there and turning the wheel back. When the speed and speedLat (which depends on lane coordinates, not vehicle coordinates) stay constant, I expect the angle to have a constant part. If we change the lanechange.duration to higher values, this constant part should become larger and vice versa.
  • The vehicle is still accelerating here. After the acceleration reaches zero, the angle stays constant.
  1. Or did I missunderstand something here?

sublane_model/strategic/overtake_stopped_on_the_left

I do not know what happened the last commits, but my results look different:
image
and
image
This also shows the problem from above, computeAngle() is calculated with the SpeedLat-value from the previous step when the lane change already finished. (this is done here, because isChangingLanes() does not exist for the sublane model and therefore lcm.setSpeedLat(0.0); is never set.)

Third question: what tests are good to look at to see the improvement of the new code? Or do we need more tests?

There should be improvements for most of the tests. You could also add the tests posted in #8529.

Independent of the previous questions, I found another issue: getBackPosition() does not work correctly with the sublane model, because isChangingLanes() seems to only exist for the continuous model.

  1. Is this true?

Please tell me how/where I can help for a fast integration.

@Domsall
Copy link
Contributor Author

Domsall commented Nov 22, 2022

@namdre: a short reminder, if this got stuck in the stack :)

I can also divide this issue into smaller ones. Anything that helps getting this fixed

@namdre
Copy link
Contributor

namdre commented Nov 22, 2022

Hi @Domsall, indeed the issue got buried in the stack and since it has quite a big impact on many tests it's not easily squeezed into my work queue. If you can separate out different aspects into distinct PRs that would probably help.

(i.e.

  • a changeset that only affects the final angle update (if you can put some plots into the PR that show the improvements in before/after angle timelines that would be awesome too)
  • a changeset that integrates the new angle computation code (same with the plots)
  • A new ticket and discussion for the getBackPosition issue.

To answer at least an easy question:

The issue I am having is: When the lane change ends, SpeedLat and the Angle get calculated one last time in the LC-part of the Code. In the next step, SpeedLat is still "active" in the Angle calculation, because the Angle gets updated in the CF-part and SpeedLat gets updated after that in the LC-part of the Code. computeAngle() could be added to the LC-part. But then it would always be calculated twice (or we delete it from the CF-part).

Right now the sublane model does update the angle a second time and I'd prefer it if the continuous model would do the same (with a final calculation once the maneuver is completed).

PS: I'm also aware of #11583 ...

@Domsall
Copy link
Contributor Author

Domsall commented Nov 22, 2022

Right now the sublane model does update the angle a second time and I'd prefer it if the continuous model would do the same (with a final calculation once the maneuver is completed).

From what I understand, the angle already gets updated a second time in both models, but only during the LC-maneuver. When the maneuver finishes, the angle does not get updated in the LC Code part anymore (in both models).
The issue in an example:

  1. timestep 0.1s, PosLat 0.05m, SpeedLat 0.5m/s, Angle gets calculated twice
  2. timestep 0.2s, PosLat 0.00m, SpeedLat 0.5m/s, Angle gets calculated twice
  3. timestep 0.3s, PosLat 0.00m, SpeedLat 0m/s, Angle gets calculated once (and in my case with SpeedLat 0.5m/s, because SpeedLat changes in the LC part of the Code and the Angle gets calculated before that)

The solution from my previous post: computeAngle() could be added to the LC-part. But then it would always be calculated twice (or we delete it from the CF-part).

@namdre
Copy link
Contributor

namdre commented Nov 22, 2022

I have another idea: we can delay the call to lcm.setSpeedLat(0.0); until the next step so it won't affect the output of the step where the change was concluded.
Just put it into MSVehicle::computeAngle:

if (!lcm.isChangingLanes()) {     
        lcm.setSpeedLat(0.0);     
 }
result += lefthandSign * myLaneChangeModel->calcAngleOffset()

in the next step when the angle is being computed, we can simp

@Domsall
Copy link
Contributor Author

Domsall commented Nov 22, 2022

yes, that is a good idea and brings me to the last issue: The sublane-model does not update the variable myLaneChangeCompletion, which is used in isChangingLanes().

I will create a new issue for this one.

@namdre
Copy link
Contributor

namdre commented Nov 22, 2022

This is because the sublane model can dynamically change the direction and distance while conducting a maneuver. Correspondingly it doesn't use a completion value but only tracks the remaining maneuver distance myManeuverDist.

@Domsall
Copy link
Contributor Author

Domsall commented Nov 22, 2022

I fully understand this, but how should the previous if-clause look to also work for the sublane model?! What is the sublane counterpart?

@namdre
Copy link
Contributor

namdre commented Nov 22, 2022

getManeuverDist() == 0 should do the trick (though there might be some lose ends where it isn't fully reset yet).

namdre added a commit that referenced this pull request Nov 23, 2022
…ore the next angle computation. refs #11882, alternativ to fix #12139
@Domsall Domsall closed this Dec 1, 2022
@Domsall Domsall deleted the issue8529 branch December 1, 2022 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants