Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add regression test for n-link pendulum model #278

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add regression test for n-link pendulum model #278

wants to merge 3 commits into from

Conversation

oliverlee
Copy link
Contributor

This commit resolves #108.

@oliverlee
Copy link
Contributor Author

Regression test is failing, likely due to tolerance errors near zero. We currently test the integration output using numpy.allclose with rtol=1e-7, atol=0.

@moorepants Do you have a suggestion on absolute tolerance?

@moorepants
Copy link
Member

The atol is needed to compare values to zero and the rtol for comparing to any other values.

We may need an atol value if comparisons to zero are happening.

@oliverlee oliverlee force-pushed the add-n-link-pendulum-regression-test branch from 3e71cdf to 29b1698 Compare October 17, 2015 20:02
@oliverlee
Copy link
Contributor Author

@moorepants The test still fails after adding absolute tolerance 1e-7.
Can you try this on your machine? And create the pendulum data file by uncommenting this line: https://github.com/oliverlee/pydy/blob/8ebad6af6cda935fac7265cc577f426be844b44d/pydy/tests/test_models.py#L122

This passes on my machine and I don't know how (or if I can) create the data on a Travis machine.

@oliverlee oliverlee force-pushed the add-n-link-pendulum-regression-test branch 2 times, most recently from 24f0eaf to a78282e Compare October 18, 2015 09:50
@moorepants
Copy link
Member

Yeah, this is tricky. I'll have to look at it tomorrow. Too much pydy already today.

@oliverlee oliverlee force-pushed the add-n-link-pendulum-regression-test branch 2 times, most recently from 372188c to c903bcc Compare October 19, 2015 10:07
@oliverlee
Copy link
Contributor Author

I don't know if you guys were aware, but there's a pretty large difference when using the different integrator methods

benchmark-states

benchmark-error

for state 0 simulation exceeds 1e10*eps difference between indices: [421, 999]
for state 1 simulation exceeds 1e10*eps difference between indices: [419, 999]
for state 2 simulation exceeds 1e10*eps difference between indices: [456, 999]
for state 3 simulation exceeds 1e10*eps difference between indices: [505, 982]
for state 4 simulation exceeds 1e10*eps difference between indices: [203, 999]
for state 5 simulation exceeds 1e10*eps difference between indices: [201, 991]
for state 6 simulation exceeds 1e10*eps difference between indices: [385, 999]
for state 7 simulation exceeds 1e10*eps difference between indices: [375, 962]
1e10*eps = 2.220446049250313e-06

If we want agreement with higher precision, we need to reduce simulation time by about a tenth:

for state 0 simulation exceeds 10*eps difference between indices: [131, 999]
for state 1 simulation exceeds 10*eps difference between indices: [131, 999]
for state 2 simulation exceeds 10*eps difference between indices: [203, 999]
for state 3 simulation exceeds 10*eps difference between indices: [151, 982]
for state 4 simulation exceeds 10*eps difference between indices: [111, 999]
for state 5 simulation exceeds 10*eps difference between indices: [73, 991]
for state 6 simulation exceeds 10*eps difference between indices: [200, 999]
for state 7 simulation exceeds 10*eps difference between indices: [138, 962]
1e6*eps = 2.220446049250313e-10

Are you guys okay with that for this regression test?

@oliverlee
Copy link
Contributor Author

Okay after changing the integration settings (reducing time step and reducing total simulation time) , it seems to have a much closer match between the different backends:

benchmark-error

benchmark-states

so hopefully this works on Travis.

@oliverlee
Copy link
Contributor Author

If you need to create the same plots, you can use this: https://gist.github.com/oliverlee/5a9d3e44d57313982e4b

@oliverlee
Copy link
Contributor Author

This still fails due to what I assume to be differences in integrator implementations in different versions of Python/SciPy.

For Python 2.7 oldest:

  File "/home/travis/build/pydy/pydy/pydy/tests/test_models.py", line 126, in test_n_link_pendulum_on_cart_regression
    np.testing.assert_allclose(x[:, :n+1], expected_x[:, :n+1], rtol, atol)
  File "/home/travis/miniconda/envs/test-env/lib/python2.7/site-packages/numpy/testing/utils.py", line 1347, in assert_allclose
    verbose=verbose, header=header)
  File "/home/travis/miniconda/envs/test-env/lib/python2.7/site-packages/numpy/testing/utils.py", line 708, in assert_array_compare
    raise AssertionError(msg)
AssertionError: 
Not equal to tolerance rtol=1e-07, atol=1e-07
(mismatch 0.00749999999999%)

Python 3.3 oldest:

AssertionError: 
Not equal to tolerance rtol=1e-07, atol=1e-07
(mismatch 100.0%)

Python 3.4 oldest:

AssertionError: 
Not equal to tolerance rtol=1e-07, atol=1e-07
(mismatch 15.417500000000004%)

Python 3.4 latest:

AssertionError: 
Not equal to tolerance rtol=1e-07, atol=1e-07
(mismatch 17.117500000000007%)

Python 2.7 master:

AssertionError: 
Not equal to tolerance rtol=1e-07, atol=1e-07
(mismatch 17.08%)

Python 3.4 master:

AssertionError: 
Not equal to tolerance rtol=1e-07, atol=1e-07
(mismatch 17.055000000000007%)

@oliverlee oliverlee force-pushed the add-n-link-pendulum-regression-test branch from f94add4 to 7b4b945 Compare October 19, 2015 14:41
@moorepants
Copy link
Member

This may be tough to get working. Maybe reducing the integration to something very simple would be best. The three backends that evaluate the eom's could certainly output numerically different answers. Tracking down the numerical inconsistency will be hard. I don't have any great suggestions right now for this issue.

@oliverlee oliverlee force-pushed the add-n-link-pendulum-regression-test branch from 7b4b945 to efc968d Compare October 22, 2015 16:53
@moorepants
Copy link
Member

This is probably worth rebasing/merging with master and running tests again.

@oliverlee
Add regression test for n-link pendulum model
30b5416 
This commit resolves pydy#108.
@oliverlee
Set absolute tolerance for n-link pendulum test
931d81e
@oliverlee
Change simulation parameters and test tolerances
30eac7f 
Reduce total simulation time and decrease simulation time step so that
we do not compare states after the system trajectory has diverged due to
chaotic behavior. Allow larger tolerances in comparison of system
generalized speeds.
@oliverlee oliverlee force-pushed the add-n-link-pendulum-regression-test branch from efc968d to 30eac7f Compare May 30, 2017 07:45
@oliverlee
Copy link
Contributor Author

oliverlee commented May 30, 2017 •
edited
Loading

Works on my end after rebasing off master.

dev) oliver@canopus:~/repos/pydy/pydy/tests$ nosetests test_models.py
...F
======================================================================
FAIL: pydy.tests.test_models.test_n_link_pendulum_on_cart_regression
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/oliver/miniconda3/envs/dev/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/Users/oliver/repos/pydy/pydy/tests/test_models.py", line 126, in test_n_link_pendulum_on_cart_regression
    np.testing.assert_allclose(x[:, :n+1], expected_x[:, :n+1], rtol, atol)
  File "/Users/oliver/miniconda3/envs/dev/lib/python3.5/site-packages/numpy/testing/utils.py", line 1411, in assert_allclose
    verbose=verbose, header=header, equal_nan=equal_nan)
  File "/Users/oliver/miniconda3/envs/dev/lib/python3.5/site-packages/numpy/testing/utils.py", line 796, in assert_array_compare
    raise AssertionError(msg)
AssertionError:
Not equal to tolerance rtol=1e-07, atol=1e-07

(mismatch 0.03249999999999886%)
 x: array([[  0.000000e+00,   1.570796e+00,   1.570796e+00,   1.570796e+00],
       [  3.000404e-07,   1.570797e+00,   1.570797e+00,   1.570797e+00],
       [  6.003211e-07,   1.570797e+00,   1.570797e+00,   1.570797e+00],...
 y: array([[  0.000000e+00,   1.570796e+00,   1.570796e+00,   1.570796e+00],
       [  3.000404e-07,   1.570797e+00,   1.570797e+00,   1.570797e+00],
       [  6.003211e-07,   1.570797e+00,   1.570797e+00,   1.570797e+00],...

----------------------------------------------------------------------
Ran 4 tests in 2.599s

FAILED (failures=1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add regression tests for the n-link pendulum model.
2 participants
@oliverlee @moorepants