osrf / lrauv

Packages for simulating the Tethys-class Long-Range AUV (LRAUV) from the Monterey Bay Aquarium Research Institute (MBARI).
Apache License 2.0
58 stars 13 forks source link

Revisit `lrauv_ignition_plugins` testing suite #189

Closed hidmic closed 2 years ago

hidmic commented 2 years ago

Motivation

Integration tests have been broken for a while now. Recent attempts to fix them have been met with modelling limitations, upstream changes, inadequate tuning, etc (see #187). To optimize our time, we need to clearly define what use cases we want to validate in CI, update our tests to match expectations, and ensure our code base passes.

Definition of Done

hidmic commented 2 years ago

FYI @arjo129. I take that YoYoCircle, PitchMass at a depth, and PitchAndDepthMassVBS are a must (at the bare minimum).

hidmic commented 2 years ago

After going over all tests in lrauv_ignition_plugins, I think the following expectations summarize their intent.



Note that for a test suite to validate those expectations, only mission control tests would need the LRAUV controller. Moreover, no missions would be required for test controllers. While this is not mandatory --many actuator tests currently use the controller in one way or another--, decoupling means the scope of tests (and their failure modes) is much narrower.

@arjo129 @caguero am I missing anything?

arjo129 commented 2 years ago

Hey @hidmic this set of requirements is very well documented. I think you're spot on with the Mission Control tests I'll just throw my two cents of why every test was written the way it was just to check if we are not missing anything.

lrauv_description

The tests here verify that the vehicle model has hydrostatic stability. The vehicle has a slight offset between the center of buoyancy and center of mass to ensure a restoring moment. If both these tests pass we know the vehicle has hydrostatic stability.

lrauv_ignition_plugins

Vehicle dynamics tests

This set of tests checks the behavior of our vehicle's physics in ignition. We don't integrate with the controller here as these help us identify bugs and short comings of our physics system.

Environmental

Acoustics

Integration tests

Note: These are often the flaky ones in part because of the async nature of the test. The goal is to catch errors while integrating (flipped axis, wrong assumptions).

A couple of notes on Hydrodynamics and Hydrostatics

Most obvious instabilities are caused by poor hydrostatics. During the early phase of the project we ran into lots of trouble as we overlooked the restoring moment. Tests that catch this are ones involving pitch mass etc. It should not be a problem anymore since the autogenerated model and tests in lrauv_description gaurantee this stability.

With regards to Fossen's equations they are a linear approximation. The equations are far from ideal. Our parameterization breaks down when we reverse and move too much at a pitch. With regards to the pitch this is expected as the scaling is not uniform in all axis. For the most part this should not be a problem as in real life operations the vehicle moves like a torpedo forward.

hidmic commented 2 years ago

@arjo129 thanks for the write-up!

I definitely missed the lrauv_description tests.

Note: These are often the flaky ones in part because of the async nature of the test.

Certainly, though I've noticed that often times the simulation is run asynchronously when it could have been run synchronously. To cope with mission control being asynchronous we can only restrict what the vehicle does (e.g. having the vehicle go to surface after a mission ends forces us to somehow time it).

Note: there is huge overshoot in the control of the depth in this mode. This is because of the slow fill rate of the Variable buoyancy system. We have checked this with @braanan who performed tank tests. There is both lateral translation and overshoot in real life as well.

I cannot stress enough how important this is. We should document it. Also, we should probably extend timeouts to allow controller outputs to converge.

Our parameterization breaks down when we reverse and move too much at a pitch.

This may be worth a revisit, some day. Until then, we should document this in our hydrodynamics plugin documentation.


On a related note, I've noticed a circular dependency between both packages: lrauv_ignition_plugins needs lrauv_description for tests, lrauv_description needs lrauv_ignition_plugins for modelling (and thus why we have to source the install tree before testing).

How about adding an lrauv_system_tests package that depends on both and condenses all tests? The comments in this ticket would become the documentation for that package and its tests.

arjo129 commented 2 years ago

Thanks for doing this @hidmic. Solved by #195-#199