RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.34k stars 1.26k forks source link

Numerous Matlab-based Unit Tests Fail if Certain Toolboxes are Not Installed #2017

Closed liangfok closed 7 years ago

liangfok commented 8 years ago

I noticed that if certain MATLAB toolboxes are not installed, numerous Drake unit tests will fail. Below is a summary:

Failures due to lack of the Aerospace Toolbox:

49/451 Test  #49: util/test/testQuatConjugate
Undefined function 'quatconj' for input arguments of type 'double'.
Error in testQuatConjugate (line 6)
    assert(max(abs(quatConjugate(q)-quatconj(q')')) < eps);
50/451 Test  #50: util/test/testQuatProduct
Undefined function 'quatmultiply' for input arguments of type 'double'.
Error in testQuatProduct (line 7)
    assert(max(abs(quatProduct(q1,q2)-quatmultiply(q1',q2')')) <= eps);
347/451 Test #347: examples/IRB140/runPlanning
Undefined function 'angle2quat' for input arguments of type 'double'.
Error in runPlanning (line 32)
grasp_orient = angle2quat(-1.3,1.1,0.8)';

Failures due to lack of the System Identification Toolbox:

277/451 Test #277: examples/Acrobot/test/paramEstSyntheticData
Undefined function 'iddata' for input arguments of type 'cell'.
Error in paramEstSyntheticData (line 130)
data = iddata(xsamplesfinal,usamples,Ts,'InputName',r.getInputFrame.getCoordinateNames(),'OutputName',outputFrameNames);

Failures due to lack of the Neural Network Toolbox:

344/451 Test #344: examples/HolonomicDrive/HolonomicDrivePlant.runTrajectory
Undefined function 'gadd' for input arguments of type 'double'.
Error in HolonomicDriveVisualizer/draw (line 41)
      body = gadd(x(1:2), rotation * obj.outline);
345/451 Test #345: examples/HolonomicDrive/HolonomicDrivePlant.runOpenLoop
Undefined function 'gadd' for input arguments of type 'double'.
Error in HolonomicDriveVisualizer/draw (line 41)
      body = gadd(x(1:2), rotation * obj.outline);

Failures due to lack of the Symbolic Math Toolbox:

364/451 Test #364: examples/Pendulum/test/coordinateTest
Undefined function 'syms' for input arguments of type 'char'.
Error in coordinateTest (line 6)
syms q qd;

Either the above-mentioned toolboxes should be listed as required dependencies or the unit test execution logic should be updated to automatically remove the above-mentioned unit tests when the required toolboxes are not installed.

Since the above-mentioned toolboxes are quite pricey (e.g., the standard Neural Network, Symbolic Math, System Identification, and Aerospace toolboxes are $1000 each), it may be better to make them optional dependencies.

liangfok commented 8 years ago

I first noticed this problem while trying to debug https://github.com/RobotLocomotion/drake/pull/1898.

liangfok commented 8 years ago

@david-german-tri: I assigned this to you in the hopes that you would know who could best resolve this issue.

jamiesnape commented 8 years ago

I would suggest guarding the tests with a CMake variable along the lines of USE_MATLAB_AEROSPACE_TOOLBOX and set it to ON by default for now.

RussTedrake commented 8 years ago

Let's not add more complexity to cmake options for this. These are all silly things that are easily resolved. The immediate "fix" is to throw an error with the MissingDependency syntax used by checkDependency.

Also -- these are just for tests, and the build servers are ok, so i don't consider it an emergency.

jamiesnape commented 8 years ago

Note I said variable, not option (this is an internal implementation detail).

RussTedrake commented 8 years ago

Suggesting that someone would pass it in on the command line?

jamiesnape commented 8 years ago

They could if they really needed to in the short term, but the idea would be to grab the list of installed toolboxes at CMake configure time and set the variables appropriately.

david-german-tri commented 8 years ago

Since @RussTedrake says these dependencies are easily removed, now that we have MATLAB at TRI, I'll see if I can squash some of them as an introductory exercise. @liangfok, I'll start from the bottom of your list and work up, in case you're working from the top down.

liangfok commented 8 years ago

Thanks @david-german-tri. I'm currently debugging another unit test that I believe is a true positive: https://github.com/RobotLocomotion/drake/pull/1898#issuecomment-207042156. I will return to this after I squash the bugs that I believe are real.

david-german-tri commented 8 years ago

SGTM. Useful thing for assessing the extent of a MATLAB infection, by the way: [flist, plist] = matlab.codetools.requiredFilesAndProducts('/path/to/file.m')

liangfok commented 8 years ago

@david-german-tri, I tried executing the matlab.codetools.requiredFilesAndProducts() method on:

For both, the result was:

plist = 

             Name: 'MATLAB'
          Version: '9.0'
    ProductNumber: 1
          Certain: 1

What does this tell us?

david-german-tri commented 8 years ago

@liangfok That's not what I get. Did you already clean up coordinateTest locally?


Trial>> [flist, plist] = matlab.codetools.requiredFilesAndProducts('/Users/davidgerman/drake-distro/drake/examples/Pendulum/test/coordinateTest.m')

flist = 

    '/Users/davidgerman/drake-distro/drake/examples/Pendulum/test/coordinateTest.m'

plist = 

1x2 struct array with fields:

    Name
    Version
    ProductNumber
    Certain

Trial>> plist.Name

ans =

MATLAB

ans =

Symbolic Math Toolbox
liangfok commented 8 years ago

@david-german-tri: I did not modify coordinateTest.m. Could this be a MATLAB version issue? Here's what I'm using:

>> ver
----------------------------------------------------------------------------------------------------
MATLAB Version: 9.0.0.341360 (R2016a)
MATLAB License Number: XXXX
Operating System: Mac OS X  Version: 10.11.4 Build: 15E65 
Java Version: Java 1.7.0_75-b13 with Oracle Corporation Java HotSpot(TM) 64-Bit Server VM mixed mode
----------------------------------------------------------------------------------------------------
MATLAB                                                Version 9.0         (R2016a)
Simulink                                              Version 8.7         (R2016a)
Control System Toolbox                                Version 10.0        (R2016a)
Curve Fitting Toolbox                                 Version 3.5.3       (R2016a)
Optimization Toolbox                                  Version 7.4         (R2016a)
Simulink 3D Animation                                 Version 7.5         (R2016a)
Statistics and Machine Learning Toolbox               Version 10.2        (R2016a)
david-german-tri commented 8 years ago

Interesting. Apparently requiredFilesAndProducts can't tell you about products you don't have.

http://www.mathworks.com/help/matlab/ref/matlab.codetools.requiredfilesandproducts.html

david-german-tri commented 8 years ago

2026 fixes the bottom three. The System Identification Toolkbox one looks unpleasantly baked-in, since the iddata structure is an input to an actual production function in Drake. Haven't looked at the Aerospace Toolbox quaternion stuff.

RussTedrake commented 8 years ago

I think @hongkai-dai might be able to resolve the quaternion stuff super quickly if we need help.

the iddata is a real dependency on the system id toolbox. we should use checkDependency at the top of that file.

hongkai-dai commented 8 years ago

Sorry I just saw this thread. Will work on the quaternion stuff.

david-german-tri commented 7 years ago

Closing as wontfix.