Create vehicle from XML database: check that there are not unused parameters

[juanmanzanero]

When a vehicle is loaded from XML, a nice feature to have would be that the program checks that all the inputs present in the XML file have been used, and to throw an exception instead.

Having this feature could help to identify typos in the parameters that might lead to catastrophic events. For example, if one writes <front-tyre/> instead of <front-tire/> , the whole data would just be ignored and the tire parameters wouldn't be loaded.

A way of doing this, would be that every time an XML element is read, make the software add an XML attribute _parameter_used="true". Then, after the car has been loaded, check that all the elements have this attribute set to true.

[kvkpraneeth]

Hi! I want to contribute to this repository. I would like to put a PR for this issue if it is not currently assigned.

[juanmanzanero]

Hi,

You can give it a try. First of all, read Contributing.md and pull_request_template.md

This issue has two parts.

The first one is a PR to my other software that fastest-lap rests on, lioncpp, to the file database_parameters.h, to the function read_parameters.

Basically, this function takes an Xml_document and a path to an XML element. For example, the path to <c/>

<a>
   <b>
     <c> 1.0 </c>
     <d> 2.0 </d>
   </b>
</a>

would be path="a/b/c", and the path to <d/> would be path="a/b/d"

Hence, every call to read_parameter should add the attribute __unused__="false" to the parameter read and all its parents

For example, if the parameter with path a/b/c is read, the Xml_document should read after the call:

<a __unused__="false">
   <b __unused__="false">
     <c __unused__="false"> 1.0 </c>
     <d> 2.0 </d>
   </b>
</a>

Try to implement this, create a new test in database_parameters_test.cpp, and submit a PR.

Feel free to ask anything

Cheers

[juanmanzanero]

Hey @kvkpraneeth , how long you estimate this will take you? Cheers

[juanmanzanero]

Right, step 2:

Add a call to database_parameters_all_used(database).get_root_element()) in the body of the constructor of Dynamic_model_car: (file: dynamic_model_car.h)

    //! Constructor from parameter map and tire types
    //! @param[in] parameters: parameters map
    //! @param[in] front_left_tire_type: type of the front left tire 
    //! @param[in] front_right_tire_type: type of the front right tire 
    //! @param[in] rear_left_tire_type: type of the rear left tire 
    //! @param[in] rear_right_tire_type: type of the rear right tire 
    //! @param[in] road: the road
    Dynamic_model_car(Xml_document& database, const RoadModel_t& road = RoadModel_t()) 
        : _chassis(database), _road(road) 
    { 
       if ( !database_parameters_all_used(database).get_root_element()) )
       {
            std::ostringstream s_out;
            s_out << "[ERROR] Dynamic_model_car -> not all parameters were used" << std::endl;
            database.print(s_out);
            throw fastest_lap_exception(s_out.str());
       }
    }
};

And, add a test that fails when an extra parameter is present in limebeer2014f1_ctest.cpp

To add a test that "succeeds" when an exception is thrown, you use:

try
{
    // ......code lines that throw an exception
    FAIL();
}
catch (fastest_lap_exception& ex)
{
     SUCCEED();
}
catch(...)
{
     FAIL();
}

[kvkpraneeth]

Since add_attribute has been changed to set_attribute in lion, it is breaking the build of this project, would you like me to separately change that or add it into the same PR?

[juanmanzanero]

Yeah, I had other issues in the project after other changes I made and I took to oportunity to update it there. Thanks!

[kvkpraneeth]

Hey! so I think we need to add a feature that excludes certain parameters from the function. \ Reason: The following tests fail

Test Summary

``` 5: [ FAILED ] Car_road_cartesian_test.state_vector_sizes 5: [ FAILED ] Car_road_cartesian_test.copy_constructor_frame_trees_sanity 5: [ FAILED ] Car_road_cartesian_test.copy_assignment_frame_trees_sanity 5: [ FAILED ] Car_road_cartesian_test.chassis_position 5: [ FAILED ] Car_road_cartesian_test.chassis_velocity 5: [ FAILED ] Car_road_cartesian_test.front_axle_position 5: [ FAILED ] Car_road_cartesian_test.rear_axle_position 5: [ FAILED ] Car_road_cartesian_test.front_axle_velocity 5: [ FAILED ] Car_road_cartesian_test.rear_axle_velocity 5: [ FAILED ] Car_road_cartesian_test.front_left_tire_position 5: [ FAILED ] Car_road_cartesian_test.front_right_tire_position 5: [ FAILED ] Car_road_cartesian_test.rear_left_tire_position 5: [ FAILED ] Car_road_cartesian_test.rear_right_tire_position 5: [ FAILED ] Car_road_cartesian_test.front_axle_deformations 5: [ FAILED ] Car_road_cartesian_test.rear_axle_deformations 5: [ FAILED ] Car_road_cartesian_test.front_axle_deformations_velocity 5: [ FAILED ] Car_road_cartesian_test.rear_axle_deformations_velocity 5: [ FAILED ] Car_road_cartesian_test.front_left_tire_velocity 5: [ FAILED ] Car_road_cartesian_test.front_right_tire_velocity 5: [ FAILED ] Car_road_cartesian_test.rear_left_tire_velocity 5: [ FAILED ] Car_road_cartesian_test.rear_right_tire_velocity 5: [ FAILED ] Car_road_cartesian_test.total_force 5: [ FAILED ] Car_road_cartesian_test.total_torque 5: [ FAILED ] Car_road_cartesian_test.newton_equations 5: [ FAILED ] Car_road_cartesian_test.euler_equations 5: [ FAILED ] Car_road_cartesian_test.sizes 5: [ FAILED ] Car_road_cartesian_test.dqdt 5: [ FAILED ] Car_road_cartesian_test.acceleration_simulation 5: [ FAILED ] Car_road_cartesian_test.braking_simulation 5: [ FAILED ] Car_road_cartesian_test.jacobian_autodiff 5: [ FAILED ] Car_road_cartesian_test.jacobian_autodiff_integrated_function 5: [ FAILED ] Car_road_cartesian_test.autodifftest 5: [ FAILED ] Car_road_curvilinear_test.state_vector_sizes 5: [ FAILED ] Car_road_curvilinear_test.curvilinear_road_test 5: [ FAILED ] Car_road_curvilinear_test.dqdt_test 5: [ FAILED ] Car_road_curvilinear_test.corner_simulation ```

This happens because of the extra engine parameters (gear-ratio, power-data, rpm-data) triggering the exception under the case: https://github.com/juanmanzanero/fastest-lap/blob/b167325df00711f7031b103e3e0f821ca4a1b599/src/core/actuators/engine.hpp#L12-L23

So, we need this new feature to circumvent this problem, when we purposefully do not read certain parameters.

[juanmanzanero]

Generally, to implement a feature such that a test passes is not a good idea. I disagree to implement this feature. Instead, the tests should be adapted to comply with the new rules. If parameters are passed to the constructor that are not used, then remove them from the Xml_document.

If the problem is that there are some parameters which are read but they are not declared as DECLARE_PARAMS() as it looks, then the edit is to add here:

 else 
 { 
     _gear_ratio = database.get_element(path+"gear-ratio").get_value(double()); 

     std::vector<scalar> speed = database.get_element(path+"rpm-data").get_value(std::vector<double>())*RPM; 
     std::vector<scalar> p = database.get_element(path+"power-data").get_value(std::vector<double>())*CV; 
     _p = sPolynomial(speed,p,speed.size()-1,true); 
 } 

the database.get_element(path+"gear-ratio").set_attribute("__unused__","false"); to all the parameters that are used.

[juanmanzanero]

Thank you!