Strandbeest example #13302

joemasterjohn · 2020-05-13T19:57:21Z

Adds a parameterized strandbeest example that tests out the new URDF parsing capabilities for creating LinearBushingRollPitchYaw (#13255).

This change is

jwnimmer-tri · 2020-05-13T20:08:59Z

We have @row_xacro available in our bazel workspace. Generally for new code we should use that as part of the BUILD rules, instead of committing its output into git. I can help with that integration.

The xacro data is large; did you write it on your own? Or was it copied from somewhere? (If so, we need to preserve its copyright and licensing.)

joemasterjohn

Ok thanks, I'd appreciate your help with the @xacrointegration.

I actually adapted the xacro from data from an old drake commit:
https://github.com/RobotLocomotion/drake/tree/last_sha_with_original_matlab/drake/examples/Strandbeest

Things like link names, dimensions and joint associations are the same. But there is a lot added for calculating mass/inertia accurately and the custom tags added to replace loop_joints.

Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers

jwnimmer-tri · 2020-05-13T20:20:07Z

Wow, see d87d931 -- maybe Robin actually wrote these by hand originally? I assumed we'd copied them from somewhere else. It might be nice to add Co-authored-by on the commit with the xacro, or even curate a first commit with just the file added unmodified from the few years ago. Then it would also call out your edits as diffs.

amcastro-tri · 2020-05-13T22:04:06Z

@joemasterjohn did a lot of editing to this model to get physical properties such as mass and rotational inertias right (we found out a must to get time scales right). I agree though we should give both Joe and Robin credit for this model.

jwnimmer-tri · 2020-05-14T01:43:35Z

=> #13304 provides what I hope are sufficient bazel xacro macros to generate the sdf on demand.

joemasterjohn

+@avalenzu for feature review on Monday, please!

Reviewable status: LGTM missing from assignee avalenzu, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on @avalenzu)

avalenzu

Review pass done. PTAL.

Reviewed 7 of 7 files at r1.
Reviewable status: 10 unresolved discussions, LGTM missing from assignee avalenzu, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @joemasterjohn)

examples/multibody/strandbeest/BUILD.bazel, line 28 at r1 (raw file):

    test_rule_timeout = "moderate",
    deps = [
        "//common:find_resource",

nit: Could you add //common:add_text_logging_gflags to the deps here? That enables the --spdlog_level flag. See https://github.com/RobotLocomotion/drake/blob/master/common/add_text_logging_gflags.cc.

examples/multibody/strandbeest/BUILD.bazel, line 36 at r1 (raw file):

        "//multibody/tree",
        "//solvers",
        "//solvers:snopt_solver",

nit: This is redundant, as snopt_solver is already included in //solvers.

examples/multibody/strandbeest/README.md, line 1 at r1 (raw file):

# Strandbeest

This file should probably have an acknowledgment of Theo Jansen (the creator of the Strandbeest).

examples/multibody/strandbeest/README.md, line 4 at r1 (raw file):

## Change the model
Modfiy any relevant values in `model/Macros.xacro`. The Strandbeest model is

nit: s/Modifiy/Modify

examples/multibody/strandbeest/README.md, line 9 at r1 (raw file):

## Run the example
bazel run --config snopt //examples/multibody/strandbeest:run_with_motor

Does the IK solve in run_with_motor only work with SNOPT? If not, it would be better to leave out the --config snopt.

examples/multibody/strandbeest/run_with_motor.cc, line 140 at r1 (raw file):

  }

  // Set the penetration allowand and stiction tolerance to values that make

nit: s/allowand/allowance

examples/multibody/strandbeest/run_with_motor.cc, line 162 at r1 (raw file):

  auto diagram = builder.Build();

  // Create a context for this system and context for the strandbeest system.

nit: This would probably be clearer as "... for the diagram and extract the context for the ..."

examples/multibody/strandbeest/run_with_motor.cc, line 207 at r1 (raw file):

        strandbeest.GetForceElement<LinearBushingRollPitchYaw>(bushing_index);

    ik.AddPointToPointDistanceConstraint(bushing.frameA(), Eigen::Vector3d(),

I thought that Eigen's default constructors left data uninitialized?

examples/multibody/strandbeest/model/LegAssembly.xacro, line 158 at r1 (raw file):

    <frame name="${prefix}_loop_f_g_bushing_frameA"
        link="${prefix}_bar_f"
        rpy="-1.5707963267948966 0 0"

Could these super precise constants be replaced with ${pi/2} as was done earlier in this file?

examples/multibody/strandbeest/model/LegPair.xacro, line 69 at r1 (raw file):

    <frame name="${prefix}_leg1_loop_a_c_bushing_frameA"
        link="${prefix}_bar_a"
        rpy="-1.5707963267948966 0 0"

Could this be ${-pi/2} here and below?

joemasterjohn

Thanks @avalenzu! The IK does solver with the default chosen solver without added --config snopt (which is IPOPT), but I have been running it with SNOPT because it is considerably faster. I thought that IPOPT might be causing the timeout failure I'm seeing in CI, but even after changing the test timeout to moderate it seems to timeout on two builds. Any ideas?

Reviewable status: 5 unresolved discussions, LGTM missing from assignee avalenzu, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @avalenzu)

examples/multibody/strandbeest/BUILD.bazel, line 28 at r1 (raw file):