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

fixes bug that prevent autodiff with simulate #475

Merged
merged 1 commit into from
Aug 13, 2018
Merged

fixes bug that prevent autodiff with simulate #475

merged 1 commit into from
Aug 13, 2018

Conversation

blandry
Copy link
Contributor

@blandry blandry commented Aug 11, 2018

No description provided.

@tkoolen tkoolen self-requested a review August 11, 2018 02:29
tkoolen
tkoolen approved these changes Aug 11, 2018
View reviewed changes
Copy link
Collaborator

@tkoolen tkoolen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good catch.

Do note that RigidBodySim provides more full-featured simulation capabilities including variable step size integrators, while still being autodiffable.

@blandry
Copy link
Contributor Author

blandry commented Aug 11, 2018

Great that's good to know. The tests pass on my computer (mac) which is consistent with travis (linux build only is failing). I don't want to just reduce the precision of that test if you suspect there's something more subtle but it looks like that might just be it.

@tkoolen
Copy link
Collaborator

tkoolen commented Aug 11, 2018

I'll look into the test failure later today.

tkoolen added a commit that referenced this pull request Aug 13, 2018
@tkoolen
Fix random seeds for tests.
1d0e5d7 
Should have done this a long time ago. Makes it easier to debug test failures on CI like the one in #475. Should really not be using the global RNG anyway, but requires a fix for #459. Use different seeds between test sets to avoid testing the same mechanism in all tests.
@tkoolen tkoolen mentioned this pull request Aug 13, 2018
Merged
@codecov-io
Copy link

codecov-io commented Aug 13, 2018 •
edited
Loading

Codecov Report

Merging #475 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #475   +/-   ##
=======================================
  Coverage   99.17%   99.17%           
=======================================
  Files          28       28           
  Lines        1582     1582           
=======================================
  Hits         1569     1569           
  Misses         13       13
Impacted Files Coverage Δ
src/simulate.jl 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ca24f4...5007c69. Read the comment docs.

@tkoolen tkoolen merged commit 5d4fbe4 into JuliaRobotics:master Aug 13, 2018
@tkoolen
Copy link
Collaborator

tkoolen commented Aug 13, 2018

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkoolen tkoolen tkoolen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@blandry @tkoolen @codecov-io