Cenic viz

[rpoyner-tri]

The top commit on this branch is a quick sketch of how to maybe adapt the current viz config stack to work with continuous integration/plants.

---

This change is 

[jwnimmer-tri]

@jwnimmer-tri partially reviewed 6 files and made 1 comment.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @rpoyner-tri).

---

geometry/drake_visualizer.cc line 554 at r2 (raw file):

  if (params_.publish_period > 0.0) {
    this->DeclarePeriodicPublishEvent(params_.publish_period, 0.0,

Bold idea -- possibly LeafSystem::DeclarePeriodicPublishEvent(period=0, ...) should declare a per-step event automatically? That would save us from adding this kind of branch in all visualizers (i.e., no code changes). Plausibly that might help a bunch of other third-party systems as well that need the same thing to happen?

It is admittedly slightly magical, but if it comes up so frequently maybe the magic is warranted? Maybe we ask Sherm to weigh in later on.

[rpoyner-tri]

@rpoyner-tri made 1 comment.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @rpoyner-tri).

---

geometry/drake_visualizer.cc line 554 at r2 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Bold idea -- possibly LeafSystem::DeclarePeriodicPublishEvent(period=0, ...) should declare a per-step event automatically? That would save us from adding this kind of branch in all visualizers (i.e., no code changes). Plausibly that might help a bunch of other third-party systems as well that need the same thing to happen?

It is admittedly slightly magical, but if it comes up so frequently maybe the magic is warranted? Maybe we ask Sherm to weigh in later on.

\cc @sherm1 for consideration on you return.

[rpoyner-tri]

@rpoyner-tri made 1 comment.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @rpoyner-tri).

---

geometry/drake_visualizer.cc line 554 at r2 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

\cc @sherm1 for consideration on you return.

This usage, though magical, is analogous to the semantics of mbp's time_step.

[rpoyner-tri]

+(status: defer ci)

@rpoyner-tri made 1 comment.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @rpoyner-tri).

[sherm1]

@sherm1 made 1 comment.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on rpoyner-tri).

---

geometry/drake_visualizer.cc line 554 at r2 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

This usage, though magical, is analogous to the semantics of mbp's time_step.

In my experience it is rare that anyone wants to visualize every step of a variable-step integrator, except maybe for debugging. That causes time to advance irregularly, typically with horrible slowdowns during contact events. Much more common is to visualize with steps that are fixed in simulated time rather than by the arbitrary schedule of the integrator's chosen steps. Then playback shows how the simulation really advances in time.

IMO it would be better to default to some reasonable publishing interval like 30ms of sim time with a per-step "advanced" option for those interested in the under-the-hood operation of the integrator.

[jwnimmer-tri]

@jwnimmer-tri made 1 comment.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on rpoyner-tri).

---

geometry/drake_visualizer.cc line 554 at r2 (raw file):

In my experience it is rare that anyone wants to visualize every step of a variable-step integrator, except maybe for debugging.

Yes, exactly. In the common case, a fixed-rate visualization is what we want.

However, this PR is specifically for debugging as you say. The CENIC unit test wanted its visualization to publish at exactly the integrator steps, so that the user could see every step, and so that the fixed-rate publish didn't affect the error-controlled step schedule.

So, setting the period to zero would be used rarely, but in cases where it is desired having the framework provide the logic instead of copy-pasting it into every single visualization system seemed like a win to me.

[sherm1]

@sherm1 made 1 comment.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on rpoyner-tri).

---

geometry/drake_visualizer.cc line 554 at r2 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

In my experience it is rare that anyone wants to visualize every step of a variable-step integrator, except maybe for debugging.

Yes, exactly. In the common case, a fixed-rate visualization is what we want.

However, this PR is specifically for debugging as you say. The CENIC unit test wanted its visualization to publish at exactly the integrator steps, so that the user could see every step, and so that the fixed-rate publish didn't affect the error-controlled step schedule.

So, setting the period to zero would be used rarely, but in cases where it is desired having the framework provide the logic instead of copy-pasting it into every single visualization system seemed like a win to me.

I see. Yes, documenting period=0 as a per-step advanced feature makes sense to me, and having it work that way for all visualizers seems better than per-viz fixes.

[rpoyner-tri]

@rpoyner-tri made 1 comment.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @rpoyner-tri).

---

geometry/drake_visualizer.cc line 554 at r2 (raw file):

Previously, sherm1 (Michael Sherman) wrote…

I see. Yes, documenting period=0 as a per-step advanced feature makes sense to me, and having it work that way for all visualizers seems better than per-viz fixes.

I'll work on a (likely new PR) patch that has those properties.