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.

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

Keep Acrobot model files in sync #5408

Closed
jwnimmer-tri opened this issue Mar 4, 2017 · 9 comments
Closed

Keep Acrobot model files in sync #5408

jwnimmer-tri opened this issue Mar 4, 2017 · 9 comments

Comments

@jwnimmer-tri
Copy link
Collaborator

In https://github.com/RobotLocomotion/drake/tree/master/drake/examples/Acrobot, we have Acrobot.sdf and Acrobot.urdf. The files should be interchangeable, so we should ensure that the two files remain sync'd, in terms of the resulting MultibodyTree and their dynamics.

We had a program that performed this check at https://github.com/RobotLocomotion/drake/blob/e8ecbe96a6608b0ab61390b3dce06b6dc3fe4bfe/drake/multibody/rigid_body_system1/test/compareRigidBodySystems.cpp, but it got overtaken by events. We should consider reimplementing it, or else removing one of the two duplicated model files, or etc.

@amcastro-tri
Copy link
Contributor

The acrobot benchmark will probably prove useful in verifying the two different models.

@amcastro-tri
Copy link
Contributor

cc'ing @clalancette

@EricCousineau-TRI
Copy link
Contributor

Per #11009 / #11011, I want to delete examples/Acrobot.{urdf,sdf} and only use the benchmark versions.

Once the examples models are deleted / fully slated for deprecation (and not used at all), we can close this issue.

@jwnimmer-tri
Copy link
Collaborator Author

jwnimmer-tri commented Mar 25, 2019

@RussTedrake cares a lot about what the examples look like. I suggest getting his explicit approval on any plan that touches examples/acrobot, before merging any PRs.

@RussTedrake
Copy link
Contributor

Just answered in #10294 . There is downstream code that depends on these (in underactuated).

@jwnimmer-tri
Copy link
Collaborator Author

Due to #6619 and #10294, I should probably own this issue as well.

@jwnimmer-tri
Copy link
Collaborator Author

Re-assigning to @mitiguy to resolve. In #13593 we're apparently adding yet another acrobot, buried inline in a unit test. That's beyond what I can retrofit.

@jwnimmer-tri jwnimmer-tri assigned mitiguy and unassigned jwnimmer-tri Sep 28, 2020
@mitiguy
Copy link
Contributor

mitiguy commented Sep 29, 2020

@jwnimmer-tri I am not creating another acrobot in PR #13593. I think it important to be able to create small hand-crafted fixtures/models (e.g., a kinematic-only double pendulum) for testing purposes without reliance on other models that have other dependencies.

To make this point clearer, I am adding the following comment to the test fixture (model):
// Note: The applied forces (such as gravity) on this system are irrelevant as
// the plan for this text fixture is limited to testing kinematics for an
// instantaneous given state. The mass/inertia properties of this plant are
// irrelevant and there are no actuators, sensors, controller, or ports.

Note: As you deem best, feel free to unassign this issue to me.

@jwnimmer-tri
Copy link
Collaborator Author

I'll just close this as a duplicate of #10294. I'm not sure that the SDF vs URDF sync is the big deal, vs the C++ implementations. If we do care about the files in particular, we can have it as a subtask of #10294.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants