Implementation of Traffic Scenario 01

[SubhashiniPerumal]

• Added new scenario in control_loss.py
• Added new atomic SteerVehicle
• Added new criteria ReachedLocationTest
• Added new scenario to list in scenario_runner.py

---

This change is 

[fabianoboril]

Reviewed 3 of 4 files at r1.
Reviewable status: 3 of 4 files reviewed, 5 unresolved discussions (waiting on @SubhashiniPerumal, @fabianoboril, and @germanros1987)

---

ScenarioManager/atomic_scenario_behavior.py, line 534 at r1 (raw file):

        new_status = py_trees.common.Status.RUNNING

        if CarlaDataProvider.get_velocity(self.vehicle) > EPSILON:

Why do you make the maneuver velocity dependent? It should be atomic, so if you need to have a velocity > 0, this should be another check performed before....

---

ScenarioManager/atomic_scenario_behavior.py, line 538 at r1 (raw file):

            new_status = py_trees.common.Status.SUCCESS
        else:
            new_status = py_trees.common.Status.FAILURE

Why failure? This would mean, in any case this behavior terminates after only one tick (either with success or with failure). Is this intended? If yes, you should simply set new_status to success and apply the desired steering value.

---

ScenarioManager/atomic_scenario_behavior.py, line 545 at r1 (raw file):

        self.vehicle.apply_control(self.control)

        return new_status

You have to add a terminate method, where you reset the steering control value.

---

Scenarios/control_loss.py, line 45 at r1 (raw file):

    # other vehicle parameter
    other_vehicles = []

This should come from the base class, or not?

---

Scenarios/control_loss.py, line 90 at r1 (raw file):

            jitterAction.add_child(turn)
            if i == 0:
                jitterAction.add_child(TimeOut(0.5))

Given the defined behavior, you don't need the timeout...

[fabianoboril]

Reviewed 1 of 4 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @SubhashiniPerumal and @germanros1987)

[fabianoboril]

I am not sure, if this scenario is what we really want to have.
The crash scenario describes a situation, where a vehicle lost control, due to road conditions, driving without attention, etc. Hence, I think the scenario should test, if a vehicle can drive properly from A to B (going through some narrow curves, etc.). This is also what is stated by the NHTSA crash report:
https://www.nhtsa.gov/sites/nhtsa.dot.gov/files/pre-crash_scenario_typology-final_pdf_version_5-2-07.pdf (page 29)

Typical Scenario: Vehicle is turning left or right at an
intersection-related area, in daylight, under clear weather
conditions, with a posted speed limit of 45 mph or less, and then
loses control due to wet or slippery roads and runs off the road

What the implementation does in contrast, is to manipulate the driving commands (like a tire puncture would do when driving). I am not sure, if this is what we should test here. In addition, there might be timing issues with this implementation: Assume you have two clients (the scenario + an AD stack). Both are sending steering commands to the CARLA server. The behavior may be different, depending on which command arrives first...

[germanros1987]

As we discussed in today's meeting:

• The ego-vehicle needs to lose control due to X. In this case we assume the reason is condition of the road
• This is translated as a noise signal affecting steering and throttle
• The command from the ego vehicle is intercepted and modified with the noise pattern we design
• The combination of both is send through apply_control
• This "spike" of noise has a finite duration of X seconds
• The ego-vehicle needs to recover from that

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @SubhashiniPerumal and @germanros1987)

[germanros1987]

I think reviewable got crazy. I did not approve.

[germanros1987]

Reviewed 1 of 4 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @SubhashiniPerumal)

[germanros1987]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @SubhashiniPerumal)

[fabianoboril]

And, the criteria should be updated to capture, that the vehicle re-gained control (i.e. came back to correct lane, drives normally, etc.). I suggest to do check the most simple things for now:

• No crash
• At the end of the scenario in correct lane (can be checked via waypoint) --> Add a new criterion for this
  (You could even count the number of ticks, the ego vehicle is not within the correct lane, and add a threshold to define success/failure)

[SubhashiniPerumal]

Please review. Also, should we implement the code for intercepting multiple steer controls (from manual_control and scenario), combine, apply ? Right now the scenario implementation just applies control signal independent of manual_control.

Reviewable status: 0 of 13 files reviewed, 5 unresolved discussions (waiting on @fabianoboril and @germanros1987)

---

ScenarioManager/atomic_scenario_behavior.py, line 534 at r1 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

Why do you make the maneuver velocity dependent? It should be atomic, so if you need to have a velocity > 0, this should be another check performed before....

Done.

---

ScenarioManager/atomic_scenario_behavior.py, line 538 at r1 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

Why failure? This would mean, in any case this behavior terminates after only one tick (either with success or with failure). Is this intended? If yes, you should simply set new_status to success and apply the desired steering value.

Done.

---

ScenarioManager/atomic_scenario_behavior.py, line 545 at r1 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

You have to add a terminate method, where you reset the steering control value.

We could do that but then it'll have no effect on the vehicle if the control is applied for a very short instance from update to terminate calls. We could introduce a time out (thread suspend) inside the atomic to make it wait for a while so that the applied steer value has some effect or we could keep the current implementation and add a timeout with the atomic and make the user be responsible for steering reset since it is going to be controlled by an AD stack.

---

Scenarios/control_loss.py, line 45 at r1 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

This should come from the base class, or not?

Done.

---

Scenarios/control_loss.py, line 90 at r1 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

Given the defined behavior, you don't need the timeout...

Done.

[fabianoboril]

Reviewed 9 of 13 files at r2, 4 of 4 files at r3.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @fabianoboril and @SubhashiniPerumal)

---

ScenarioManager/atomic_scenario_behavior.py, line 150 at r3 (raw file):

    def __init__(self, vehicle, target_location, distance,
                 name="InTriggerDistanceToLocation"):

For the fomatting you may use the new script in Utliities.

---

Scenarios/control_loss.py, line 31 at r3 (raw file):

    """
    Implementation of "Control Loss Vehicle" (Traffic Scenario 01)

Please also mention the location of this scenario (see my latest PR)
e.g. Town01

---

Scenarios/control_loss.py, line 37 at r3 (raw file):

    # ego vehicle parameters
    ego_vehicle_model = 'vehicle.carlamotors.carlacola'

Please use leading _ to indicate private member. Please also see my recent PR

---

Scenarios/control_loss.py, line 117 at r3 (raw file):

        criteria.append(reached_region_criterion)

        return criteria

I still have to test the scenario

[SubhashiniPerumal]

Reviewable status: 9 of 14 files reviewed, 9 unresolved discussions (waiting on @fabianoboril)

---

ScenarioManager/atomic_scenario_behavior.py, line 150 at r3 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

For the fomatting you may use the new script in Utliities.

Done.

---

Scenarios/control_loss.py, line 31 at r3 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

Please also mention the location of this scenario (see my latest PR)
e.g. Town01

Done.

---

Scenarios/control_loss.py, line 37 at r3 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

Please use leading _ to indicate private member. Please also see my recent PR

Done.

---

Scenarios/control_loss.py, line 117 at r3 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

I still have to test the scenario

Please do

[fabianoboril]

Reviewed 1 of 5 files at r5, 2 of 4 files at r6.
Reviewable status: 12 of 14 files reviewed, 12 unresolved discussions (waiting on @fabianoboril and @SubhashiniPerumal)

---

Scenarios/control_loss.py, line 77 at r7 (raw file):

        jitterTimeout = TimeOut(timeout=0.1, name="Timeout for next jitter")

        for i in range(self._no_of_jitter_actions):

These jitters have very little effect in my tests. I accelerate, and do not control the vehicle when these behaviors become active (the car is just rolling with throttle=0). The car is moving only marginally. Maybe you should increase the effect of the jitter?

---

Scenarios/control_loss.py, line 98 at r7 (raw file):

        sequence.add_child(startcondition)
        sequence.add_child(jitterSequence)
        sequence.add_child(TimeOut(60))

This cannot work. The sequence will end after start + jitters + 60 seconds. But at the same time the overall timeout is 60 seconds. So this scenario will always end with a timeout.

Instead, I recommend to check if the vehicle reached a certain location (InDistanceToLocation), and then terminate.

---

Scenarios/control_loss.py, line 120 at r7 (raw file):

        criteria.append(collision_criterion)
        criteria.append(driven_distance_criterion)
        criteria.append(reached_region_criterion)

This could be covered by my previous comment.

[germanros1987]

Rebase needed.

Reviewed 1 of 13 files at r2, 2 of 4 files at r6.
Reviewable status: 13 of 14 files reviewed, 14 unresolved discussions (waiting on @fabianoboril and @SubhashiniPerumal)

---

ScenarioManager/atomic_scenario_criteria.py, line 341 at r3 (raw file):

        super(ReachedRegionTest, self).__init__(name, vehicle, 0)
        self.logger.debug("%s.__init__()" % (self.__class__.__name__))
        self.vehicle = vehicle

_vehicle

---

ScenarioManager/atomic_scenario_criteria.py, line 344 at r3 (raw file):

        self.min_x = min_x
        self.max_x = max_x
        self.min_y = min_y

_

[SubhashiniPerumal]

Reviewable status: 11 of 14 files reviewed, 14 unresolved discussions (waiting on @germanros1987 and @fabianoboril)

---

ScenarioManager/atomic_scenario_criteria.py, line 341 at r3 (raw file):

Previously, germanros1987 wrote…

_vehicle

Done.

---

ScenarioManager/atomic_scenario_criteria.py, line 344 at r3 (raw file):

Previously, germanros1987 wrote…

_

Done.

---

Scenarios/control_loss.py, line 77 at r7 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

These jitters have very little effect in my tests. I accelerate, and do not control the vehicle when these behaviors become active (the car is just rolling with throttle=0). The car is moving only marginally. Maybe you should increase the effect of the jitter?

Increased noise std. deviation to 0.3. Also, we are right now only adding steering noise but not throttle noise. Should I also add noise to throttle ?

---

Scenarios/control_loss.py, line 98 at r7 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

This cannot work. The sequence will end after start + jitters + 60 seconds. But at the same time the overall timeout is 60 seconds. So this scenario will always end with a timeout.

Instead, I recommend to check if the vehicle reached a certain location (InDistanceToLocation), and then terminate.

Done.

---

Scenarios/control_loss.py, line 120 at r7 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

This could be covered by my previous comment.

Done.

[fabianoboril]

Reviewed 3 of 3 files at r11.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @fabianoboril, @SubhashiniPerumal, and @germanros1987)

---

Scenarios/control_loss.py, line 77 at r7 (raw file):

Previously, SubhashiniPerumal wrote…

Increased noise std. deviation to 0.3. Also, we are right now only adding steering noise but not throttle noise. Should I also add noise to throttle ?

Now it is very aggressive. Maybe use something in the middle?

---

Scenarios/control_loss.py, line 56 at r11 (raw file):

                                         hero=True)

        super(ControlLoss, self).__init__(name="ControlLoss",

Somehow your commit is not based on the current master branch! Please rebase

[germanros1987]

Reviewed 1 of 12 files at r12, 3 of 3 files at r15.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @fabianoboril and @SubhashiniPerumal)

---

ScenarioManager/atomic_scenario_behavior.py, line 545 at r1 (raw file):

Previously, SubhashiniPerumal wrote…

We could do that but then it'll have no effect on the vehicle if the control is applied for a very short instance from update to terminate calls. We could introduce a time out (thread suspend) inside the atomic to make it wait for a while so that the applied steer value has some effect or we could keep the current implementation and add a timeout with the atomic and make the user be responsible for steering reset since it is going to be controlled by an AD stack.

Is this issue resolved?

[fabianoboril]

Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @fabianoboril and @SubhashiniPerumal)

---

Scenarios/control_loss.py, line 56 at r11 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

Somehow your commit is not based on the current master branch! Please rebase

There is still some problem with your rebase, or you missed something. The base class for the scenario has changed, so you need to update the call of the parent constructor!

[fabianoboril]

Reviewed 1 of 12 files at r12, 3 of 3 files at r15.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @fabianoboril and @SubhashiniPerumal)

[SubhashiniPerumal]

Reviewable status: 2 of 4 files reviewed, 12 unresolved discussions (waiting on @fabianoboril and @germanros1987)

---

ScenarioManager/atomic_scenario_behavior.py, line 545 at r1 (raw file):

Previously, germanros1987 wrote…

Is this issue resolved?

It is resolved if we decide to stick with the current approach.

---

Scenarios/control_loss.py, line 77 at r7 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

Now it is very aggressive. Maybe use something in the middle?

We have used a new dynamic mean split distribution noise which has more narrower sample magnitude band. Please check it out. There is a signal with std. dev. of 0.02 with mean +0.05 and -0.05 being applied to steering.

---

Scenarios/control_loss.py, line 56 at r11 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

There is still some problem with your rebase, or you missed something. The base class for the scenario has changed, so you need to update the call of the parent constructor!

Yes, realized that. Fixed.

[fabianoboril]

@SubhashiniPerumal Thx for the updates. I tested it, and it looks much better than before. Still, I am bit worried about the usability of this scenario. It happened a few times, that the vehicle crashed against street lights, while the control loss was active. If we think about German's idea for the challenge, the scenario will always "win" against the client/user driving commands. So, the car crashes, because of the scenario, not because the user code could not handle the situation. In other words, the scenario will cause the failure, not the user code.

@germanros1987, @SubhashiniPerumal : Maybe we need to move this scenario to Town03, where the streets are wider? Or the control_loss needs to be updated, such the car doesn't hit a street light...
Suggestions?

[fabianoboril]

Reviewed 2 of 2 files at r16.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @fabianoboril and @SubhashiniPerumal)

---

ScenarioManager/atomic_scenario_criteria.py, line 369 at r16 (raw file):

                self.test_status = "RUNNING"

        if self.test_status == "SUCCESS":

I am not sure, if this is what you want. Imagine the car is inside the region, but then leaves it afterwards. This will not be recognized by your code. Is this intended?

[SubhashiniPerumal]

We could tune the noise strength and narrow it down even further but since the road is so narrow, pretty much any disturbance well push the vehicle out of the lane at least by a little and since the street lights are so close, we might end up hitting one easily. May be it is a good idea to move to a road that is a little wider. What do you say @germanros1987 ?

Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @fabianoboril)

---

ScenarioManager/atomic_scenario_criteria.py, line 369 at r16 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

I am not sure, if this is what you want. Imagine the car is inside the region, but then leaves it afterwards. This will not be recognized by your code. Is this intended?

Yes. We want to check for the vehicle coming back on to the correct lane. The criteria that we have created does a single detection of whether the vehicle reached a specified region (in our case, a region on the correct lane after experiencing loss of control) or not.

[germanros1987]

Yes, it seems a good idea. Let's move it to Town03

Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @fabianoboril)

---

ScenarioManager/atomic_scenario_criteria.py, line 369 at r16 (raw file):

Previously, SubhashiniPerumal wrote…

Yes. We want to check for the vehicle coming back on to the correct lane. The criteria that we have created does a single detection of whether the vehicle reached a specified region (in our case, a region on the correct lane after experiencing loss of control) or not.

Sounds reasonable

[SubhashiniPerumal]

Moved the scenario to Town03

Reviewable status: 1 of 4 files reviewed, 13 unresolved discussions (waiting on @fabianoboril and @germanros1987)

[fabianoboril]

Reviewed 3 of 3 files at r20.
Reviewable status: complete! all files reviewed, all discussions resolved

[fabianoboril]

Code is ok. Please update Docs/list_of_supported_scenarios.md!

[SubhashiniPerumal]

Updated the documentation.

Reviewable status: 0 of 5 files reviewed, all discussions resolved (waiting on @fabianoboril and @germanros1987)

[fabianoboril]

Please rebase

[fabianoboril]

Reviewed 1 of 4 files at r21, 4 of 4 files at r23.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @SubhashiniPerumal)

---

ScenarioManager/atomic_scenario_behavior.py, line 208 at r23 (raw file):

            self._vehicle) - self._target_velocity
        if delta_velocity < EPSILON:
=======

Please resolve this merge conflict

[fabianoboril]

Reviewed 1 of 1 files at r24.
Reviewable status: complete! all files reviewed, all discussions resolved

[SubhashiniPerumal]

Reviewable status: complete! all files reviewed, all discussions resolved

---

ScenarioManager/atomic_scenario_behavior.py, line 208 at r23 (raw file):

Previously, fabianoboril (Fabian Oboril) wrote…

Please resolve this merge conflict

Resolved the conflicts

[germanros1987]

rebase again.

Reviewable status: complete! all files reviewed, all discussions resolved

[SubhashiniPerumal]

rebased again. kindly check

Reviewable status: 1 of 6 files reviewed, all discussions resolved (waiting on @fabianoboril)

[fabianoboril]

Reviewed 1 of 5 files at r25, 4 of 4 files at r27.
Reviewable status: complete! all files reviewed, all discussions resolved