From 0ea6152c350fa029f3670b0464237484c6a5196d Mon Sep 17 00:00:00 2001
From: Jeremy Nimmer <jeremy.nimmer@tri.global>
Date: Mon, 31 Aug 2020 10:58:29 -0700
Subject: [PATCH] schema: Move rotation and transform out of dev (#13949)

---
 common/schema/BUILD.bazel                     | 46 +++++++++++++++
 common/schema/dev/BUILD.bazel                 | 59 -------------------
 common/schema/{dev => }/rotation.cc           |  2 +-
 common/schema/{dev => }/rotation.h            |  0
 common/schema/stochastic.h                    | 29 ++++++---
 common/schema/{dev => }/test/rotation_test.cc |  2 +-
 common/schema/test/stochastic_test.cc         | 22 +++++++
 .../schema/{dev => }/test/transform_test.cc   |  7 ++-
 common/schema/{dev => }/transform.cc          |  2 +-
 common/schema/{dev => }/transform.h           | 14 ++---
 multibody/parsing/dev/BUILD.bazel             |  2 +-
 multibody/parsing/dev/model_directives.h      |  2 +-
 .../parsing/dev/process_model_directives.cc   |  2 +-
 13 files changed, 106 insertions(+), 83 deletions(-)
 delete mode 100644 common/schema/dev/BUILD.bazel
 rename common/schema/{dev => }/rotation.cc (98%)
 rename common/schema/{dev => }/rotation.h (100%)
 rename common/schema/{dev => }/test/rotation_test.cc (98%)
 rename common/schema/{dev => }/test/transform_test.cc (92%)
 rename common/schema/{dev => }/transform.cc (98%)
 rename common/schema/{dev => }/transform.h (95%)

diff --git a/common/schema/BUILD.bazel b/common/schema/BUILD.bazel
index 27eb3c1ed2f0..80ec6270b0f6 100644
--- a/common/schema/BUILD.bazel
+++ b/common/schema/BUILD.bazel
@@ -15,7 +15,20 @@ drake_cc_package_library(
     name = "schema",
     visibility = ["//visibility:public"],
     deps = [
+        ":rotation",
         ":stochastic",
+        ":transform",
+    ],
+)
+
+drake_cc_library(
+    name = "rotation",
+    srcs = ["rotation.cc"],
+    hdrs = ["rotation.h"],
+    deps = [
+        ":stochastic",
+        "//common:name_value",
+        "//math:geometric_transform",
     ],
 )
 
@@ -32,13 +45,46 @@ drake_cc_library(
     ],
 )
 
+drake_cc_library(
+    name = "transform",
+    srcs = ["transform.cc"],
+    hdrs = ["transform.h"],
+    deps = [
+        ":rotation",
+        ":stochastic",
+        "//common:name_value",
+        "//math:geometric_transform",
+    ],
+)
+
 # === test/ ===
 
+drake_cc_googletest(
+    name = "rotation_test",
+    deps = [
+        ":rotation",
+        "//common/test_utilities:eigen_matrix_compare",
+        "//common/yaml:yaml_read_archive",
+        "//common/yaml:yaml_write_archive",
+    ],
+)
+
 drake_cc_googletest(
     name = "stochastic_test",
     deps = [
         ":stochastic",
         "//common/test_utilities:eigen_matrix_compare",
+        "//common/test_utilities:expect_throws_message",
+        "//common/test_utilities:symbolic_test_util",
+        "//common/yaml:yaml_read_archive",
+    ],
+)
+
+drake_cc_googletest(
+    name = "transform_test",
+    deps = [
+        ":transform",
+        "//common/test_utilities:eigen_matrix_compare",
         "//common/test_utilities:symbolic_test_util",
         "//common/yaml:yaml_read_archive",
     ],
diff --git a/common/schema/dev/BUILD.bazel b/common/schema/dev/BUILD.bazel
deleted file mode 100644
index b7243604f813..000000000000
--- a/common/schema/dev/BUILD.bazel
+++ /dev/null
@@ -1,59 +0,0 @@
-# -*- mode: python -*-
-# vi: set ft=python :
-
-load(
-    "@drake//tools/skylark:drake_cc.bzl",
-    "drake_cc_googletest",
-    "drake_cc_library",
-)
-load("//tools/lint:lint.bzl", "add_lint_tests")
-
-package(default_visibility = ["//visibility:private"])
-
-drake_cc_library(
-    name = "rotation",
-    srcs = ["rotation.cc"],
-    hdrs = ["rotation.h"],
-    deps = [
-        "//common:name_value",
-        "//common/schema:stochastic",
-        "//math:geometric_transform",
-    ],
-)
-
-drake_cc_library(
-    name = "transform",
-    srcs = ["transform.cc"],
-    hdrs = ["transform.h"],
-    visibility = ["//multibody/parsing/dev:__pkg__"],
-    deps = [
-        ":rotation",
-        "//common:name_value",
-        "//common/schema:stochastic",
-        "//math:geometric_transform",
-    ],
-)
-
-# === test/ ===
-
-drake_cc_googletest(
-    name = "rotation_test",
-    deps = [
-        ":rotation",
-        "//common/test_utilities:eigen_matrix_compare",
-        "//common/yaml:yaml_read_archive",
-        "//common/yaml:yaml_write_archive",
-    ],
-)
-
-drake_cc_googletest(
-    name = "transform_test",
-    deps = [
-        ":transform",
-        "//common/test_utilities:eigen_matrix_compare",
-        "//common/test_utilities:symbolic_test_util",
-        "//common/yaml:yaml_read_archive",
-    ],
-)
-
-add_lint_tests()
diff --git a/common/schema/dev/rotation.cc b/common/schema/rotation.cc
similarity index 98%
rename from common/schema/dev/rotation.cc
rename to common/schema/rotation.cc
index 7bac193ef99e..96de6a04f3b4 100644
--- a/common/schema/dev/rotation.cc
+++ b/common/schema/rotation.cc
@@ -1,4 +1,4 @@
-#include "drake/common/schema/dev/rotation.h"
+#include "drake/common/schema/rotation.h"
 
 #include "drake/common/drake_throw.h"
 #include "drake/math/random_rotation.h"
diff --git a/common/schema/dev/rotation.h b/common/schema/rotation.h
similarity index 100%
rename from common/schema/dev/rotation.h
rename to common/schema/rotation.h
diff --git a/common/schema/stochastic.h b/common/schema/stochastic.h
index da7cae82d066..c616222eb7f8 100644
--- a/common/schema/stochastic.h
+++ b/common/schema/stochastic.h
@@ -20,14 +20,14 @@ namespace schema {
 @{
 
 This page describes how to use classes such as schema::Distribution to denote
-stochastic quantities, as bridge between loading a scenario specification and
+stochastic quantities, as a bridge between loading a scenario specification and
 populating the corresponding symbolic::Expression quantities into a
 systems::System.
 
-<h1>Stochastic variables</h1>
+# Stochastic variables
 
-We'll explain uses of schema::Distribution and related using the matching YAML
-syntax as parsed by yaml::YamlReadArchive.
+We'll explain uses of schema::Distribution and related types using the matching
+YAML syntax as parsed by yaml::YamlReadArchive.
 
 Given this C++ data structure:
 
@@ -101,13 +101,13 @@ These one-line spellings are the equivalent to those above.
   value: !UniformDiscrete { values: [1.0, 1.5, 2.0] }
 ```
 
-<h1>Vectors of stochastic variables</h1>
+# Vectors of stochastic variables
 
 For convenience, we also provide the option to specify a vector of independent
 stochastic variables with the same type.
 
-We'll explain uses of schema::DistributionVector and related using the matching
-YAML syntax as parsed by yaml::YamlReadArchive.
+We'll explain uses of schema::DistributionVector and related types using the
+matching YAML syntax as parsed by yaml::YamlReadArchive.
 
 Given this C++ data structure:
 
@@ -188,7 +188,7 @@ others by specifying a zero-sized range for the constant elements:
     std: [0.0, 1.0]
 ```
 
-<h1>See also </h1>
+# See also
 
 See @ref schema_transform for one practical application, of specifying
 rotations, translations, and transforms using stochastic schemas.
@@ -383,6 +383,15 @@ class DeterministicVector final : public DistributionVector {
 };
 
 /// A gaussian distribution with vector `mean` and vector or scalar `stddev`.
+///
+/// When `mean` and `stddev` both have the same number of elements, that
+/// denotes an elementwise pairing of the 0th mean with 0th stddev, 1st mean
+/// with 1st stddev, etc.
+///
+/// Alternatively, `stddev` can be a vector with a single element, no matter
+/// the size of `mean`; that denotes the same `stddev` value applied to every
+/// element of `mean`.
+///
 /// @tparam Size rows at compile time (max 6) or else Eigen::Dynamic.
 template <int Size>
 class GaussianVector final : public DistributionVector {
@@ -447,7 +456,9 @@ struct InvalidVariantSelection {
 /// Variant over all kinds of vector distributions.
 ///
 /// If the Size parameter allows for 1-element vectors (i.e, is either 1 or
-/// Eigen::Dynamic), then this variant also offers the single distributions.
+/// Eigen::Dynamic), then this variant also offers the single distribution
+/// types (Deterministic, Gaussian, Uniform).  If the Size parameter is 2 or
+/// greater, the single distribution types are not allowed.
 ///
 /// @tparam Size rows at compile time (max 6) or else Eigen::Dynamic.
 template <int Size>
diff --git a/common/schema/dev/test/rotation_test.cc b/common/schema/test/rotation_test.cc
similarity index 98%
rename from common/schema/dev/test/rotation_test.cc
rename to common/schema/test/rotation_test.cc
index 8bac856fdba9..acddf1733108 100644
--- a/common/schema/dev/test/rotation_test.cc
+++ b/common/schema/test/rotation_test.cc
@@ -1,4 +1,4 @@
-#include "drake/common/schema/dev/rotation.h"
+#include "drake/common/schema/rotation.h"
 
 #include <gtest/gtest.h>
 
diff --git a/common/schema/test/stochastic_test.cc b/common/schema/test/stochastic_test.cc
index 44efb23fe36e..6ac76a3db9f7 100644
--- a/common/schema/test/stochastic_test.cc
+++ b/common/schema/test/stochastic_test.cc
@@ -6,6 +6,7 @@
 #include <gtest/gtest.h>
 
 #include "drake/common/test_utilities/eigen_matrix_compare.h"
+#include "drake/common/test_utilities/expect_throws_message.h"
 #include "drake/common/test_utilities/symbolic_test_util.h"
 #include "drake/common/yaml/yaml_read_archive.h"
 
@@ -348,6 +349,27 @@ GTEST_TEST(StochasticTest, ZeroSizeRandomRanges) {
       Eigen::VectorXd::Constant(1, 1.5)));
 }
 
+struct FixedSize2 {
+  template <typename Archive>
+  void Serialize(Archive* a) {
+    a->Visit(DRAKE_NVP(value));
+  }
+
+  DistributionVectorVariant<2> value;
+};
+
+GTEST_TEST(StochasticTest, IncorrectVectorTest) {
+  const char* const input = R"""(
+value: !Deterministic { value: 2.0 }
+)""";
+  FixedSize2 parsed;
+  DRAKE_EXPECT_THROWS_MESSAGE(
+      YamlReadArchive(YAML::Load(input), kStrict).Accept(&parsed),
+      std::exception,
+      ".*unsupported type tag !Deterministic while selecting a variant<> entry"
+      " for std::variant<.*> value.*");
+}
+
 }  // namespace
 }  // namespace schema
 }  // namespace drake
diff --git a/common/schema/dev/test/transform_test.cc b/common/schema/test/transform_test.cc
similarity index 92%
rename from common/schema/dev/test/transform_test.cc
rename to common/schema/test/transform_test.cc
index 6a99d44dc557..74b27bce1947 100644
--- a/common/schema/dev/test/transform_test.cc
+++ b/common/schema/test/transform_test.cc
@@ -1,4 +1,4 @@
-#include "drake/common/schema/dev/transform.h"
+#include "drake/common/schema/transform.h"
 
 #include <gtest/gtest.h>
 
@@ -17,6 +17,7 @@ namespace {
 // once Drake's YamlReadArchive constructor default changes to be strict.
 constexpr YamlReadArchive::Options kStrict;
 
+// TODO(jwnimmer-tri) Change use R""" per Drake GSG, throughout this file.
 const char* deterministic = R"R(
 base_frame: foo
 translation: [1., 2., 3.]
@@ -97,6 +98,8 @@ GTEST_TEST(StochasticSampleTest, TransformTest) {
   drake::math::RigidTransformd sampled_transform = transform.Sample(&generator);
 
   // The sampled transforms must lie within the randomization domain.
+  // TODO(jwnimmer-tri) The correctness checks below traceable to rotation.cc's
+  // implementation would be better placed in rotation_test.cc.
   const auto& translation_domain =
       std::get<schema::UniformVector<3>>(transform.translation);
   for (int i = 0; i < 3; i++) {
@@ -117,7 +120,7 @@ GTEST_TEST(StochasticSampleTest, TransformTest) {
   EXPECT_LT(rpy[2], rotation_domain.max[2]);
   EXPECT_GT(rpy[2], rotation_domain.min[2]);
 
-  // A second sample will (virtually certainly) differ from the first sample.
+  // A second sample will (almost certainly) differ from the first sample.
   const drake::math::RigidTransformd sampled_transform_2 =
       transform.Sample(&generator);
   EXPECT_FALSE(sampled_transform.IsExactlyEqualTo(sampled_transform_2));
diff --git a/common/schema/dev/transform.cc b/common/schema/transform.cc
similarity index 98%
rename from common/schema/dev/transform.cc
rename to common/schema/transform.cc
index d1e5a37440c7..7306139b13df 100644
--- a/common/schema/dev/transform.cc
+++ b/common/schema/transform.cc
@@ -1,4 +1,4 @@
-#include "drake/common/schema/dev/transform.h"
+#include "drake/common/schema/transform.h"
 
 #include <stdexcept>
 
diff --git a/common/schema/dev/transform.h b/common/schema/transform.h
similarity index 95%
rename from common/schema/dev/transform.h
rename to common/schema/transform.h
index 802dbef847e2..c1217083e14d 100644
--- a/common/schema/dev/transform.h
+++ b/common/schema/transform.h
@@ -4,7 +4,7 @@
 #include <string>
 
 #include "drake/common/name_value.h"
-#include "drake/common/schema/dev/rotation.h"
+#include "drake/common/schema/rotation.h"
 #include "drake/common/schema/stochastic.h"
 #include "drake/math/rigid_transform.h"
 
@@ -16,8 +16,8 @@ namespace schema {
 @{
 
 This page describes how to use classes such as schema::Rotation and
-schema::Transform to denote stochastic quantities, as bridge between loading a
-scenario specification and populating the corresponding math::RigidTransform
+schema::Transform to denote stochastic quantities, as a bridge between loading
+a scenario specification and populating the corresponding math::RigidTransform
 quantities.
 
 The broader concepts are discussed at @ref schema_stochastic. Here, we cover
@@ -26,7 +26,7 @@ details related to rotations and transforms in particular.
 We'll explain uses of schema::Rotation and schema::Transform using their
 matching YAML syntax as parsed by yaml::YamlReadArchive.
 
-<h1>Rotations</h1>
+# Rotations
 
 This shows the syntax for a schema::Rotation.
 
@@ -70,7 +70,7 @@ rotation: !Rpy { deg: [10.0, 20.0, 30.0] }
 rotation: !AngleAxis { angle_deg: 10.0, axis: [0.0, 1.0, 0.0] }
 ```
 
-<h2>Stochastic Rotations</h2>
+## Stochastic Rotations
 
 To specify a stochastic rotation sampled from a uniform distribution over
 SO(3):
@@ -104,7 +104,7 @@ rotation: !AngleAxis
 For an explanation of `!Uniform`, `!UniformVector`, and other available options
 (%Gaussian, etc.) for scalar and vector quantities, see @ref schema_stochastic.
 
-<h1>Transforms</h1>
+# Transforms
 
 This shows the syntax for a schema::Transform.  A transform is merely a
 translation and rotation, optionally with a some given string as a base_frame.
@@ -124,7 +124,7 @@ translation and rotation, optionally with a some given string as a base_frame.
   rotation: !Identity {}
 ```
 
-<h2>Stochastic Transforms</h2>
+## Stochastic Transforms
 
 Either or both of the rotational or translation component can be stochastic:
 
diff --git a/multibody/parsing/dev/BUILD.bazel b/multibody/parsing/dev/BUILD.bazel
index 8270b45d3a6a..3e986ec91088 100644
--- a/multibody/parsing/dev/BUILD.bazel
+++ b/multibody/parsing/dev/BUILD.bazel
@@ -48,7 +48,7 @@ drake_cc_library(
     deps = [
         "//common:essential",
         "//common:name_value",
-        "//common/schema/dev:transform",
+        "//common/schema:transform",
         "//math:geometric_transform",
     ],
 )
diff --git a/multibody/parsing/dev/model_directives.h b/multibody/parsing/dev/model_directives.h
index 5679c01edb2a..ce9c8fa300cd 100644
--- a/multibody/parsing/dev/model_directives.h
+++ b/multibody/parsing/dev/model_directives.h
@@ -11,7 +11,7 @@
 
 #include "drake/common/eigen_types.h"
 #include "drake/common/name_value.h"
-#include "drake/common/schema/dev/transform.h"
+#include "drake/common/schema/transform.h"
 #include "drake/common/text_logging.h"
 #include "drake/math/rigid_transform.h"
 #include "drake/math/roll_pitch_yaw.h"
diff --git a/multibody/parsing/dev/process_model_directives.cc b/multibody/parsing/dev/process_model_directives.cc
index c8d7fc3c47c2..b0434bd3c2b6 100644
--- a/multibody/parsing/dev/process_model_directives.cc
+++ b/multibody/parsing/dev/process_model_directives.cc
@@ -7,7 +7,7 @@
 
 #include "drake/common/filesystem.h"
 #include "drake/common/find_resource.h"
-#include "drake/common/schema/dev/transform.h"
+#include "drake/common/schema/transform.h"
 #include "drake/common/yaml/yaml_read_archive.h"
 #include "drake/multibody/parsing/parser.h"