particles example doesn't visualize because it incorrectly uses AllocateContext

[huweiATgithub]

What happened?

As I have raised in https://github.com/RobotLocomotion/drake-external-examples/issues/233, I found that the particle example did not draw in the visualizer. The example in the drake-external-examples repo is the same as that in the drake repo: https://github.com/RobotLocomotion/drake/tree/master/examples/particles.

I was noticing that neither the OutputPose callback of the geometry nor the OutputState callback of the particle system (connected to geometry) was being called during the simulation. Besides, a simple anchored geometry registered to the scene_graph is also not showing. However, the state of the particle and the calculating of the time derivative is working properly.

After some diggings and comparison with the scene_graph examples, I finally managed to have the particle moved and shown correctly by replacing builder.BuildInto(this); in the constructor of UniformlyAcceleratedParticle with builder.Build(); (and having the simulation inside the constructor).

It seems that the BuildInto method might have some problems in handling stuff with scene_graph.

Version

No response

What operating system are you using?

Ubuntu 20.04

What installation option are you using?

apt install drake

Relevant log output

No response

[SeanCurtis-TRI]

I believe the error can be found here:

https://github.com/RobotLocomotion/drake/blob/master/examples/particles/uniformly_accelerated_particle.cc#L81

If you change the call from AllocateContext() to CreateDefaultContext() the geometry is then put to work. This is because the geometry data doesn't get copied into the context by allocation.

[SeanCurtis-TRI]

Specifically, this points to a bug in SceneGraph -- it most likely arose when the geometry data stopped being state and became a parameter. The constructed model only gets copied over into the context as a result of calling SetDefaultParameters() (which is called by CreateDefaultContext()). It also needs to be copied when simply allocating the context.

[jwnimmer-tri]

My vote is that we simply delete the particle example.

I do think we should have an example which demonstrates dynamics without using a MultibodyPlant, along with illustration geometry -- but I think both example/acrobot and example/pendulum serve that purpose just fine already. A particle that accelerates beyond the visualizer within a moment of the start of the simulation isn't terribly useful, to me.

[SeanCurtis-TRI]

That's fine and good.

However, I introduced an elephant into the room. The claim that SceneGraph is broken. However, as a result of #14189 and as noted in the release notes for v0.24.0, it is not/should not be the expectation that SceneGraph works when simply allocating a context. We should explicitly be calling CreateDefaultContext().

So, in addition to uselessly showing a particle accelerate out of the view, its implementation is an anti-pattern.

I did a quick survey to see if AllocateContext() was called anywhere else bad, and it doesn't seem to be. So, nuking this one example seems to be sufficient to eliminate the anti-pattern. I'll rename the issue again and submit a PR to nuke the example in question.

[huweiATgithub]

I believe the particle example itself is not the problem. It is the simplest example that builds a dynamical system with visualization from scratch. What is wrong is the way it initializes the context. It seems that changing the way of setting up context will fix it. If AllocateContext has some problem, I think it should be either hidden from public usage or has the caveats documented properly.

[jwnimmer-tri]

That's a good point. The API documentation for AllocateContext (on both SystemBase and System) should remind users that it fills the returned context with NaNs and/or nulls, and to use CreateDefaultContext instead if they wanted meaningful values.

The purpose of "Allocate" is for advanced uses where we need to allocate storage ahead of time, for a function that needs an output-pointer to scribble into. Probably that means "(Advanced) " should precede the AllocateContext documentation as well.

[jwnimmer-tri]

It might be worth considering renaming AllocateContext to AllocateUninitializedContext for utmost clarity?

[jwnimmer-tri]

PR #17653 deleted the Drake copy of the broken demo, but not the drake-external-examples copy, e.g.: https://github.com/RobotLocomotion/drake-external-examples/tree/main/drake_cmake_installed/src/particles

There are actually a lot of copies:

jwnimmer@call-cps:~/jwnimmer-tri/drake-external-examples$ find . -name 'particle.cc' | sort
drake_ament_cmake_installed/src/drake_ament_cmake_installed/src/particle.cc
drake_catkin_installed/src/drake_catkin_installed/src/drake_catkin_installed/particle.cc
drake_cmake_installed_apt/src/particle.cc
drake_cmake_installed/src/particles/particle.cc

This recently confused a user on StackOverflow.

[jwnimmer-tri]

Ah, I was slightly confused. Only the drake_cmake_installed copy has the (broken) visualization. The other copies are just doing an example of LeafSystem dynamics with no visualization.