maliput / maliput_malidrive

Open-source ready OpenDrive backend for Maliput
BSD 3-Clause "New" or "Revised" License
1 stars 2 forks source link

Split tolerance used within xodr parsing and RoadGeometry construction process. #9

Closed agalbachicar closed 3 years ago

agalbachicar commented 4 years ago

Just what the title says.

The expected behavior is that we are can decouple those two tolerances and improve our building process and coverage.

Comes from https://github.com/ToyotaResearchInstitute/malidrive/issues/517#issuecomment-684035799

francocipollone commented 4 years ago

In order to achieve this goal, we should get all the possible information about the XODR.

Knowing the (Xs,Ys) start point, the heading and the length we could estimate the final (Xf,Yf) point for each geometry. Finally, we could calculate the difference between the endpoint of a geometry and the start point of the next geometry.

The DBManager could provide a method to get the worst case from the XODR.

agalbachicar commented 4 years ago

Agreed. Note that again, it'd be good to have both the shortest and largest gaps.

francocipollone commented 4 years ago

A small technical doubt. Early we chatted about that this analysis shouldn't be done during parsing time, but at demand when the method is called. However, I think that the builder will always want to know this information, so I think that makes sense to perform this analysis when the parser is executed and just return the outcome when the api's method is called.

agalbachicar commented 4 years ago

There is a world where this processing is not needed because we know in advance values that will work with the map (e.g. a test) so we should run this code just when it is needed. I expect it to last a considerable amount of time with respect to other tasks in parse time. Hence, the request is still valid I think.

What do you think?

francocipollone commented 4 years ago

Ah okok, If we allow passing those values then it definitely makes sense to perform this analysis on demand. I agree then. Thanks.

agalbachicar commented 4 years ago

In ToyotaResearchInstitute/malidrive#590, I started to draft a solution to derive a good enough value of tolerance that is derived from the geometry issues.

agalbachicar commented 4 years ago

I believe the tolerance automatic adjustment will work under certain circumstances but for most of real world maps the malidrive's Builder will need to be smart and solve some of the geometry descriptions. The following list describes some the cases that are problematic and we might do something to solve them:

The suggested approach for this is to:

agalbachicar commented 4 years ago

After running ToyotaResearchInstitute/malidrive#603 in a local branch to test maps, I could identify that roads that are already part of the malidrive integration tests are loaded. The following list shows the problematic xodr Road IDs in each map:

I will add tests to solve those specific cases in order.

liangfok commented 4 years ago

What do the numbers correspond to? Like, in Town01, what does "73" mean? Is it a road number?

francocipollone commented 4 years ago

That's right. It is the Road Id

agalbachicar commented 4 years ago

What do the numbers correspond to? Like, in Town01, what does "73" mean? Is it a road number?

@liangfok I have edited the comment to make it explicit that those are the XODR Road Ids.

liangfok commented 4 years ago

Thanks. Do we have visualizations of the problematic road IDs so we can see the problem?

agalbachicar commented 4 years ago

We don't have visualizations because they fail when building the RoadCurve. Failures are related to constraint violations. I will send a few PRs to easily reproduce the errors.

agalbachicar commented 4 years ago

In the meantime, I'd like to illustrate with a piece of the Road 399 in Town04:

        <planView>
            <geometry s="0.0000000000000000e+0" x="3.1283780569538402e+2" y="1.8031339835661868e+2" hdg="-1.5797139021955537e+0" length="4.3068866785660020e-2">
                <line/>
            </geometry>
            <geometry s="4.3068866785660020e-2" x="3.1283742163060742e+2" y="1.8027033120230766e+2" hdg="-1.5797139021955537e+0" length="7.0006205424934933e+0">
                <line/>
            </geometry>
            <geometry s="7.0436894092791817e+0" x="3.1277499389648438e+2" y="1.7326998901367188e+2" hdg="-1.5797139021955546e+0" length="2.6432891455440028e+0">
                <line/>
            </geometry>
            <geometry s="9.6869785548231846e+0" x="3.1275142247863840e+2" y="1.7062680496877235e+2" hdg="-1.5797139021955546e+0" length="9.3738955918390445e+0">
                <line/>
            </geometry>
            <geometry s="1.9060874146662229e+1" x="3.1266783116581797e+2" y="1.6125328209532074e+2" hdg="-1.5797139021955546e+0" length="4.1237163998090409e-2">
                <line/>
            </geometry>
        </planView>

If you take a look at the headings and lengths of the lines, you will see that the first two share the same heading as well as the last thee do. The first length and the last length is a couple of order of magnitudes less than the other lengths. When putting everything together it throws an error because the tolerance constraint.

On the other hand, the map should not be like this, a line could be created out of several lines but should not have different orientations.

agalbachicar commented 4 years ago

I will send a few PRs to easily reproduce the errors.

Enabling the simplifications in ToyotaResearchInstitute/malidrive#606

francocipollone commented 4 years ago

Comments about Town01.xodr.

Road 73 Using constants::kLinearTolerance (= 5e-2)

The XODR describes two "problematics" :

Solution I was able to sort out these issues in two ways:

One option: enable geometry simplification to solve A and then adjust tolerance to solve B.

Another option: decrease tolerance to avoid A. (B is implicitly covered too)

francocipollone commented 4 years ago

Comments about Town02.xodr.

Similar issues to Town01

Decreasing tolerance < 2.8e-4 solves that issue in Road 274 however it causes another error in Road 255


**The following just provides information about the issue:**
The latter 4 geometries of this Road are the following:
```xml
            ....
            ....
            <geometry s="1.1199366424583692e+1" x="8.9943258779271185e-1" y="-1.8955265216961777e+2" hdg="-4.5836984543057291e-4" length="2.0490863633288026e+0">
                <line/>
            </geometry>
            <geometry s="1.3248452787912495e+1" x="2.9487558380403724e+0" y="-1.8955359151766498e+2" hdg="-4.5836984543168313e-4" length="7.5101660250603608e-2">
                <line/>
            </geometry>
            <geometry s="1.3323554448163097e+1" x="3.0238574904014373e+0" y="-1.8955362594200020e+2" hdg="-4.5836984543168313e-4" length="1.1257591711339821e-1">
                <line/>
            </geometry>
            <geometry s="1.3436130365276496e+1" x="3.1364333956885719e+0" y="-1.8955367754340409e+2" hdg="-4.5836984543168313e-4" length="2.6366579807310586e-1">
                <line/>
            </geometry>

The aforementioned issue about contiguity checking happens between the first and second geometry, when simplification is disabled. If we enable the simplification feature then from the second to the fourth geometry are packed together, therefore the issue is still happening between the first geometry and second geometry, which was simplified.

francocipollone commented 4 years ago

LaneSection's length plays a role too when choosing the correct tolerance.

I used xodr_query with the last improvement(#609) in the above maps. EDIT: I've added information about the largest gap and shortest geometry too.


francocipollone commented 4 years ago

Question is. Does Road Runner also create small LaneSections as CARLA's xodr generator(apparently) does?

I tried Cloverleaf, which is a large map created by RR and:

liangfok commented 4 years ago

I've asked the question internally within TRI and will report back. Meanwhile, is "RRLongRoad" generated by RoadRunner?

francocipollone commented 4 years ago

I've asked the question internally within TRI and will report back. Meanwhile, is "RRLongRoad" generated by RoadRunner?

Thanks! Yes, RRLongRoad was generated by RoadRunner.

And also Leghorn. I've just tried the last Leghorn map that was shared with us.

Leghorn2.0

Shortest LaneSection in the XODR: 0.00319252  -- Located at RoadHaderId: 330, LaneSection Index: 0
liangfok commented 4 years ago

Alright, based on the above, I believe we can safely conclude that RoadRunner also generates short LaneSections, and that Malidrive 2.0 needs to be able to elegantly handle them.

agalbachicar commented 4 years ago

I will write here a F2F chat with @francocipollone for posterity and visibility. New additions will be added in favor of improving the build process and trying to handle malformed maps:

These two changes are expected to:

francocipollone commented 4 years ago

Regarding the new error that you discovered @agalbachicar when loading Town0X maps (but 01 and 05) which is :

terminate called after throwing an instance of 'maliput::common::assertion_error'
  what():  Failure at /home/franco/Worskpace/RoadNetwork/tri_ws/src/malidrive/src/malidrive/road_curve/road_curve_offset.cc:235 in PFromS(): condition 'dense_output->end_time() >= full_length' failed.

That condition fails because full_length is bigger than dense_output->end_time() (obviously) but for about ~1e-17 So clearly here the problem is related to the comparison between double types. In every case, the difference is about that order.

Just doing:

MALIDRIVE_THROW_UNLESS(dense_output->end_time() >= full_length - std::numeric_limits<double>::epsilon());
solves the issue.

We should decide which is the best tolerance to perform the comparison here. Once that is solved Town02, 03, 06 and 07 could be loaded (Town04 fails in another part.)



Town04 has a particularity

Road 735 has the following elevation profile:

        <elevationProfile>
            <elevation s="0.0000000000000000e+0" a="9.8075278075713044e+0" b="-1.6967480128781007e-2" c="3.8310299155731173e-6" d="0.0000000000000000e+0"/>
            <elevation s="1.0165258117922349e-5" a="9.8075276350924891e+0" b="-1.6967480050894192e-2" c="3.8310299155731173e-6" d="0.0000000000000000e+0"/>
            <elevation s="3.8398544263657186e-1" a="9.8527823517460060e+0" b="-1.6964537995022289e-2" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>
            <elevation s="1.0061806568593283e+0" a="9.7904578527171413e+0" b="-1.4212740423763861e-2" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>
            <elevation s="1.9819063267692783e+1" a="9.5230752355466244e+0" b="-1.4212740423763883e-2" c="2.7198619032002585e-4" d="0.0000000000000000e+0"/>
        </elevationProfile>

When the Piecewise Function is being created then G1contiguity is verified between individual functions. Between the second and the third elevation function, there is a considerable(6e-2) difference when is being evaluated.

lhs->f(lhs->p1()): 9.80101310706914
rhs->f(rhs->p0()): 9.85278235174601

I've checked the math and effectively the XODR is not accurate enough.

However, moving the linear tolerance to 6e-2 relaxes the checking and in consequence, it wouldn't fail.

Note: This is the first XODR map where the elevation description is a constraint in the tolerance selection.

francocipollone commented 4 years ago

Failing when running integration tests in delphyne

Adding maps to the integration tests in delphyne-gui

Some errors were found when running delphyne-mali2 with some maps. Although they are errors that happen in delphyne/delphyne-gui's namespace, I share it here because certainly could be related to malidrive's new implementation.

Traceback (most recent call last):
  File "/home/franco/Worskpace/RoadNetwork/tri_ws/install/delphyne-demos/bin/delphyne-mali2", line 9, in <module>
    delphyne_demos.demos.mali2.main()
  File "/home/franco/Worskpace/RoadNetwork/tri_ws/install/delphyne-demos/lib/python3.6/site-packages/delphyne_demos/demos/mali2.py", line 294, in main
    time_step=sim_runner_time_step
  File "/home/franco/Worskpace/RoadNetwork/tri_ws/install/delphyne/lib/python3.6/site-packages/delphyne/trees.py", line 37, in setup
    simulation=builder.build(),
IndexError: vector::_M_range_check: __n (which is 0) >= this->size() (which is 0)

 - **Highway**: (**SOLVED: Agent was being spawned on a non-driveable lane**) (EDITED)
    - Tolerance: 1e-3
    - Map is loaded (MalidriveLoad) correctly.
Fails:

EDITED

francocipollone commented 4 years ago

Town03's error

I narrowed down the error and it led me to mesh creation:

It fails within the method RenderSegment in the maliput/utilities/generate_obj.cc file when building meshes for Segment Id: 62_0.

<road name="Road 62" length="1.0030283729747680e-2" id="62" junction="-1">

I see two interesting things:

agalbachicar commented 4 years ago

Have you tried it with the simplification?

francocipollone commented 4 years ago

Have you tried it with the simplification?

Yes! Same error.

francocipollone commented 4 years ago

Town03 - Continue

I've found why it fails. Given that there are no driveable lanes within Segment Id: 62_0 an out-of-range occurs when calling segment->lane(0) in RenderSegment method in: maliput/utilites/generate_obj.cc

~I guess~ It is confirmed that Town07 fails because of the same thing.

francocipollone commented 4 years ago

RRLongRoad - continue RRLongRoad: Tolerance: 1e-3 Map is loaded correctly. Error when creating meshes. Agent's spawned at 00-4 ()

RuntimeError: Failure at /home/franco/Worskpace/RoadNetwork/tri_ws/src/malidrive/src/malidrive/road_curve/malidrive_road_curve.cc:120 in Orientation(): condition 'prh.x() >= ground_curve_->p0() && prh.x() <= ground_curve_->p1()' failed.

Relaxing the tolerance to 5e-2 this error is avoided. However, I will dig deeper here in order to avoid this throw even when the tolerance is smaller.

francocipollone commented 4 years ago

RRLongRoad - continue RRLongRoad: Tolerance: 1e-3 Map is loaded correctly. Error when creating meshes.

Error in line: https://github.com/ToyotaResearchInstitute/malidrive/blob/183948c640fe5dcf0d1e0051f71b4965b3e62cb6/src/malidrive/road_curve/malidrive_road_curve.cc#L120

Values of p0, p, and p1.

MalidriveLane id: 6_0_-4
p0: 0  p: 483.354320243623   p1: 483.353981561629

I couldn't reveal the origin of this numerical issue yet.


Appendix This Road has a particularity. It is composed of 94 geometries. https://github.com/ToyotaResearchInstitute/malidrive/blob/183948c640fe5dcf0d1e0051f71b4965b3e62cb6/resources/RRLongRoad.xodr#L969-L1258

Simplification doesn't sort out the problem.

Note: Relaxing the tolerance to 5e-2 this error is avoided.

francocipollone commented 4 years ago

Based on this comment: https://github.com/ToyotaResearchInstitute/malidrive/issues/618#issuecomment-704278288 (Issue with cloverleaf map)

The elevation and superelevation functions described in the XODR description will also play an important role when selecting an appropriate tolerance. Given that we are demanding contiguity when elevation or superelevation is piecewise-defined.

francocipollone commented 4 years ago

As we are supporting maps with a lateral shift of the road reference line ( laneOffset records of the XODR different than zero) another variable enters the equation when selecting an appropriate tolerance for the loading. Contiguity of the offset description.

I raise this because I found an interesting discontinuity in the offset description of Town03 in the RoadId 847.

<laneOffset s="0.0000000000000000e+0" a="6.6349999999999998e+0" b="0.0000000000000000e+0" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>
<laneOffset s="1.1837467941303236e+1" a="6.6349999999999998e+0" b="0.0000000000000000e+0" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>
<laneOffset s="1.4335927289340910e+1" a="6.3500000000000001e-1" b="0.0000000000000000e+0" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>

There is no need to do math here given that the three descriptions are constant (only a coefficient). One visible issue is happening between the second and the third offset description:

A major difference of about 6 units.


In cloverleaf it is also possible to find this issue: https://github.com/ToyotaResearchInstitute/malidrive/issues/618#issuecomment-705833586


Same thing happens in Town04 in the RoadId 859:

            <laneOffset s="0.0000000000000000e+0" a="0.0000000000000000e+0" b="0.0000000000000000e+0" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>
            <laneOffset s="3.8276944838791621e+1" a="1.4000000000000043e+1" b="0.0000000000000000e+0" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>
            <laneOffset s="3.8729657523174353e+1" a="1.4000000000000043e+1" b="0.0000000000000000e+0" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>

There is no continuity between the first and second polynomial.


Same thing happens int Town05 in the RoadId 1230:

            <laneOffset s="0.0000000000000000e+0" a="-3.4999999999999689e+0" b="0.0000000000000000e+0" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>
            <laneOffset s="4.8592277044036791e-2" a="-3.4999999999999689e+0" b="0.0000000000000000e+0" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>
            <laneOffset s="9.7192089618474142e-2" a="-3.4999999999999689e+0" b="0.0000000000000000e+0" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>
            <laneOffset s="1.4578435912737575e-1" a="0.0000000000000000e+0" b="0.0000000000000000e+0" c="0.0000000000000000e+0" d="0.0000000000000000e+0"/>

There is no continuity between the third and fourth polynomial.

francocipollone commented 4 years ago

At the moment, in order to summarize, the following shows the problems that the loader/builder needs to elegantly solve (if it is possible):

All these issues were obtained from testing real maps.

francocipollone commented 4 years ago

It triggers the necessity to also query the derivative of the elevation(and superelevation?). --> Related to https://github.com/ToyotaResearchInstitute/malidrive/issues/649

agalbachicar commented 3 years ago

This feature request is no longer needed. #77 was merged and we don't need to split tolerances anymore