Some Drake stochastics are not closed under `Sample()`

[ggould-tri]

Is your feature request related to a problem? Please describe. DistributionVariant and DistributionVectorVariant sample to instances of themselves -- that is, given DistributionVariant a, DistributionVariant(a.Sample()) is valid and IsDeterministic. This has the effect of making these variants closed under Sample, which is very convenient for sampling a schema-defined class (e.g. a yaml scenario).

However,

• The Rotation variant lacks this property: Rotation.Sample() -> RotationMatrix, which is not a member of Rotation.
• The Transform class is worse, since Transform.Sample()->RigidTransform not only means that the type isn't closed under Sample() but also causes #20754 (loss of base frame during sampling) because RigidTransform is semantically different from Transform.

The result of this is that generating a IsDeterministic instance from a schema-defined object requires special cases for each stochastic type rather than a single general-purpose mechanism.

Describe the solution you'd like

All Drake stochastic types, both classes and variants, should Sample() to deterministic versions of themselves rather than to separate types. Given deprecation guarantees and styleguide rules on overloading, this should be done by creating a new method rather than by changing Sample. I'd suggest SampleToDeterministic to express the invariant that this function returns the same type but guarantees that the result IsDeterministic().

SampleToDeterministic would provide the clear invariants that:

• if x.IsDeterministic() then x.SampleToDeterministic() == x
• x.SampleToDeterministic().IsDeterministic() == true
• x.SampleToDeterministic().DeterministicValue() is in distribution x

Describe alternatives you've considered

Because what I've described is more properly described as Sample, changing the existing Sample method would be cleaner. But that would require deprecating the existing method, which is slow and disruptive, or else overloading on return type, which is forbidden.

Creating an all-in-one free function to sample any stochastic type to itself would be reasonable, but the correct way to implement that would probably be to build SampleToDeterministic on every type anyway, or the equivalent through a huge visit. It would also possibly hide the solution to #20754 in a nonlocal place.

[sherm1]

Assigned to @jwnimmer-tri for disposition. I couldn't find a "component" label that seems right -- took a guess but likely wrong.

[jwnimmer-tri]

DistributionVariant and DistributionVectorVariant sample to instances of themselves -- that is, given DistributionVariant a, DistributionVariant(a.Sample()) is valid and IsDeterministic. This has the effect of making these variants closed under Sample, which is very convenient for sampling a schema-defined class (e.g. a yaml scenario).

I'm with you so far.

• The Rotation variant lacks this property: Rotation.Sample() -> RotationMatrix, which is not a member of Rotation.
• ... Transform.Sample()->RigidTransform ... isn't closed under Sample() ...

This isn't the property you asked for above. For a schema type Foo and a variable foo of that type, you asked for bar = Foo(foo.Sample()) to run without error and obey bar.IsDeterministic(). This already works for schema::Transform -- it defines constructor that takes a const RigidTransform& which means that Transform(transform.Sample()) meets the objective. The schema::Rotation is similar -- it already has the constructor you want -- but note that there isn't any Rotation::Sample() function defined, only GetDeterministicValue().

Maybe this is just a feature request for Rotation::Sample() const to exist and return a RotationMatrix<double>, to match up with Transform?

... causes #20754 (loss of base frame during sampling) because RigidTransform is semantically different from Transform.

I don't follow. The #20574 is already fixed; it throws when the sample would lose information, so there is no problem here.

All Drake stochastic types, both classes and variants, should Sample() to deterministic versions of themselves rather than to separate types.

I wasn't able to follow why think this is relevant or important. The thing we already return (the computational type, not the schema type), is the more useful answer for a sampled result. If you want to convert back to the thin schema wrapper for some unusual use case, you can already do that -- just call the schema object's constructor passing the sampled value.

SampleToDeterministic

If we do need such a function, I'd much rather we do SampleInPlace() that mutates this. That seems more like what is actually wanted, and is a nice parallel with other API in Drake.

In any case, more info is needed here because it seems I don't understand the problem.

[jwnimmer-tri]

I couldn't find a "component" label that seems right -- took a guess but likely wrong.

@sherm1 For context -- this is a file loading problem (yaml schema awkwardness stuff), so therefore the category is "parsing".