vectorgrp / OSC-NCAP-scenarios

Scenarios for testing active safety systems according to Euro NCAP Test Protocols modelled with OpenSCENARIO XML
http://www.vector.com/ncap
Other
17 stars 2 forks source link

Some notes #6

Closed eknabevcc closed 4 weeks ago

eknabevcc commented 1 month ago

First: Great contribution. Well crafted scenarios, including use of parameters and catalogs.

While updating esmini to run the scenarios I made a few notes I'd like to share and potentially discuss. Since it relates to interpretation of OpenSCENARIO XML I think this open forum is better than mail.

1. Outer scope parameters referred to within catalogs

I notices at least one case ($_Ego_speed, in ManeuverCatalog.xosc) where catalog elements make use of variables assumed to be defined in outer scope. While this is convenient and probably perfectly OK, I see two potential issues: First, it makes the catalog entry dependent on global/outer parameters. Secondly, it may lead to unintended re-use of global (outer scope) parameters. A way to avoid those cons is to declare any parameters used in the catalog. Something like this:

    <ParameterDeclarations>
        <ParameterDeclaration name="SetSpeed" parameterType="double" value="20.0"/>
    </ParameterDeclarations>

That way, the parameter has a default value (20.0 in this case) AND the user can override it from the outer scope. Something like this:

    <CatalogReference catalogName="ManeuverCatalog" entryName="ACC_Controller">
        <ParameterAssignments>
            <ParameterAssignment parameterRef="SetSpeed" value="22.0"/>
        </ParameterAssignments>
    </CatalogReference>

But, since I find this pattern in some of your other catalogs, maybe there is a special/interesting reason for the re-use of global parameter case?

Anyway, esmini accepts both ways, so no issue here.

2. Mathematical constants in expressions

I found "pi" in some expressions. And yes, it's really convenient to put "pi" instead if "3.1415...". esmini did not support it, but now (since v2.38.4) both "pi" and "e" are supported. Maybe it's obvious that these constants should be supported, otherwise it should be added explicitly to the OpenSCENARIO XML standard. E.g. in a new heading: "Supported constants". Let's discuss in v1.4 iteration.

3. laneId in routePosition, wrong sign?

This is a bit tricky. The scenario NCAP_AEB_VRU_CBNAO_2023 makes use of route "VRU_Intersection_Farside" to position one of the stationary cars (ObstructionLarge). The specified position is: FromLaneCoordinates with laneId="-1" and s="167.61" (after resolved expressions). This position happens to be in the junction, at a connecting road (9) between waypoint roads 1 and 3. The direction of road 9 is opposite to the road of first waypoint (road 1).

Now, lane -1 makes sense if considering road direction at start of the route, which is road 1 with reference line pointing towards the intersection. However, lane -1 at road 9 is at the opposite side. So the question becomes: How to interpret the laneId of given route position. I see three possibilities:

  1. Let first waypoint decide direction. Sign of any laneId relates to that direction - regardless of actual road direction at given route s value.
  2. Use actual road direction at given s-value. In other words, use laneId as is.
  3. Given laneId relates to original route lane. Example: route runs at lane -3 on road at given s-value. laneId="1" would result in -3 + 1 = -2.

I haven't thought enough to have a strong opinion. Current default implementation in esmini is according to 2. But we added an option "--align_routepositions" that will change behavior into 1.

Disadvantage of 2: It requires awareness of the road direction at given s value. Advantage of 2: After all, it is probably the most straight forward solution, causing less confusion.

I might got something wrong, or missed other possibilities. What is your thoughts on this?

jakobkaths commented 4 weeks ago

Thanks for the kudos and for checking the scenarios in esmini, @eknabevcc! Regarding your notes:

  1. We do need this outer scope for the Variables that we use for easier result analysis, but I believe we could avoid it for the Parameters without loss of functionality. I will have a closer look and probably change the implementation here.
  2. We already had the constants supported in DYNA4, so I deliberately deviated from the standard here (as it would also be an easy one for search&replace to fix for other implementations...). But I would much prefer to see this in the standard, so I created https://code.asam.net/simulation/openscenario/openscenario-xml/-/issues/672
  3. If I get you right, we use method 1. I will try to provide some detail: based on the waypoints we create the route and it has an s-direction. Given this fact, we think it is a) "fair" to assume that t (or the lane ID) must be perpendicular in the right-handed coordinate system to this path S direction and we think it is b) "unfair" to make the sign of this jump based on the underlying OpenDRIVE road network. "Unfair", because it places some burden on the user.
eknabevcc commented 4 weeks ago

Thanks for the kudos and for checking the scenarios in esmini, @eknabevcc! Regarding your notes:

  1. We do need this outer scope for the Variables that we use for easier result analysis, but I believe we could avoid it for the Parameters without loss of functionality. I will have a closer look and probably change the implementation here.
  2. We already had the constants supported in DYNA4, so I deliberately deviated from the standard here (as it would also be an easy one for search&replace to fix for other implementations...). But I would much prefer to see this in the standard, so I created https://code.asam.net/simulation/openscenario/openscenario-xml/-/issues/672

Great, I totally agree. Thanks for creating the OpenSCENARIO XML issue!

  1. If I get you right, we use method 1. I will try to provide some detail: based on the waypoints we create the route and it has an s-direction. Given this fact, we think it is a) "fair" to assume that t (or the lane ID) must be perpendicular in the right-handed coordinate system to this path S direction and we think it is b) "unfair" to make the sign of this jump based on the underlying OpenDRIVE road network. "Unfair", because it places some burden on the user.

I must agree this approach makes most sense, at least at first glance.

But I thought about it, trying to imagine use cases where the user enters the position. In some cases the signed laneId will be given, e.g. if picking the point in a graphical tool. But I can also see cases where user need to find out some other way.

The current phrasing by the standard: laneId = "Lane ID of the actual position", to me sounds more like the "true" signed laneId, as given by the OpenDRIVE road network.

Maybe we could introduce support for both ways, similar to the RelativeLanePosition dsLane vs ds to the PositionInLaneCoordinates.

I was just about to add an issue for this specific topic, but noticed that there already is one #636 which suggest to discuss this in next iteration. 👍

I will try to discuss with further colleagues as well, before permanently change esmini default behavior. I have no strong opinion myself.

Closing the issue since questions answered and remaining slight difference (route pos laneId) noted.

jakobkaths commented 4 weeks ago

Sounds good to me, @eknabevcc. The issue #636 you mentioned was reported by my colleague as we did have this discussion internally prior to implementation. Nice that you have a flag for esmini though, so I assume the scenarios run fine for you with that flag being set and your integration of "pi".

eknabevcc commented 4 weeks ago

Exactly. The scenarios all runs fine in esmini as well, at least to my understanding :) - including "pi", and with the --align_routepositions flag set.

BTW: It's just three scenarios affected by the flag - only the ones making use of FromLaneCoordinates.