Skip to content

Commit b36bc26

Browse files
rpoyner-trijwnimmer-tri
authored andcommitted
[pydrake] Align Simulator with nanobind factory constructor API
1 parent 5339545 commit b36bc26

6 files changed

Lines changed: 55 additions & 38 deletions

File tree

bindings/generated_docstrings/systems_analysis.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4340,6 +4340,11 @@ See also:
43404340
Initialize(), AdvancePendingEvents(), SimulatorStatus,
43414341
set_monitor())""";
43424342
} AdvanceTo;
4343+
// Symbol: drake::systems::Simulator::EmplaceWithSharedContext
4344+
struct /* EmplaceWithSharedContext */ {
4345+
// Source: drake/systems/analysis/simulator.h
4346+
const char* doc = R"""()""";
4347+
} EmplaceWithSharedContext;
43434348
// Symbol: drake::systems::Simulator::GetCurrentWitnessTimeIsolation
43444349
struct /* GetCurrentWitnessTimeIsolation */ {
43454350
// Source: drake/systems/analysis/simulator.h
@@ -4445,11 +4450,6 @@ Returns ``status``:
44454450
See also:
44464451
AdvanceTo(), AdvancePendingEvents(), SimulatorStatus)""";
44474452
} Initialize;
4448-
// Symbol: drake::systems::Simulator::MakeWithSharedContext
4449-
struct /* MakeWithSharedContext */ {
4450-
// Source: drake/systems/analysis/simulator.h
4451-
const char* doc = R"""()""";
4452-
} MakeWithSharedContext;
44534453
// Symbol: drake::systems::Simulator::ResetStatistics
44544454
struct /* ResetStatistics */ {
44554455
// Source: drake/systems/analysis/simulator.h

bindings/pydrake/systems/analysis_py.cc

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -291,24 +291,28 @@ PYDRAKE_MODULE(analysis, m) {
291291
auto cls = DefineTemplateClassWithDefault<Simulator<T>>(
292292
m, "Simulator", GetPyParam<T>(), doc.Simulator.doc);
293293
cls // BR
294-
.def(py::init([](const System<T>& system, py::object py_context) {
295-
// Handle the two cases for context ownership explicitly:
296-
// 1. If py_context is None, create a new context and take ownership.
297-
// 2. If py_context is provided, use the existing Python wrapper
298-
// directly (it already owns the C++ object).
299-
if (py_context.is_none()) {
300-
std::unique_ptr<Context<T>> context_ptr =
301-
system.CreateDefaultContext();
302-
// Use take_ownership because we just created this context and need
303-
// Python to own it. The unique_ptr is released, leaving the raw
304-
// pointer with no owner until take_ownership establishes Python
305-
// ownership.
306-
py_context =
307-
py::cast(context_ptr.release(), py_rvp::take_ownership);
308-
}
309-
return Simulator<T>::MakeWithSharedContext(
310-
system, make_shared_ptr_from_py_object<Context<T>>(py_context));
311-
}),
294+
.def(
295+
"__init__",
296+
[](Simulator<T>* self, const System<T>& system,
297+
py::object py_context) {
298+
// Handle the two cases for context ownership explicitly:
299+
// 1. If py_context is None, create a new context and take
300+
// ownership.
301+
// 2. If py_context is provided, use the existing Python wrapper
302+
// directly (it already owns the C++ object).
303+
if (py_context.is_none()) {
304+
std::unique_ptr<Context<T>> context_ptr =
305+
system.CreateDefaultContext();
306+
// Use take_ownership because we just created this context and
307+
// need Python to own it. The unique_ptr is released, leaving
308+
// the raw pointer with no owner until take_ownership
309+
// establishes Python ownership.
310+
py_context =
311+
py::cast(context_ptr.release(), py_rvp::take_ownership);
312+
}
313+
Simulator<T>::EmplaceWithSharedContext(self, system,
314+
make_shared_ptr_from_py_object<Context<T>>(py_context));
315+
},
312316
py::arg("system"), py::arg("context") = py::none(),
313317
// Keep alive, reference: `self` keeps `system` alive.
314318
py::keep_alive<1, 2>(),

systems/analysis/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,7 @@ drake_cc_googletest(
522522
":implicit_euler_integrator",
523523
":runge_kutta3_integrator",
524524
":simulator",
525+
"//common:scope_exit",
525526
"//common/test_utilities:expect_throws_message",
526527
"//common/test_utilities:is_dynamic_castable",
527528
"//systems/analysis/test_utilities:controlled_spring_mass_system",

systems/analysis/simulator.cc

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,10 @@ Simulator<T>::Simulator(std::unique_ptr<const System<T>> owned_system,
2222
: Simulator(nullptr, std::move(owned_system), std::move(context), true) {}
2323

2424
template <typename T>
25-
std::unique_ptr<Simulator<T>> Simulator<T>::MakeWithSharedContext(
26-
const System<T>& system, std::shared_ptr<Context<T>> context) {
27-
// This slightly odd spelling allows access to the private constructor.
28-
return std::unique_ptr<Simulator<T>>(
29-
new Simulator(&system, nullptr, std::move(context), false));
25+
void Simulator<T>::EmplaceWithSharedContext(
26+
Simulator<T>* self, const System<T>& system,
27+
std::shared_ptr<Context<T>> context) {
28+
new (self) Simulator(&system, nullptr, std::move(context), false);
3029
}
3130

3231
template <typename T>

systems/analysis/simulator.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -275,14 +275,20 @@ class Simulator {
275275
std::unique_ptr<Context<T>> context = nullptr);
276276

277277
#ifndef DRAKE_DOXYGEN_CXX
278-
// (Internal use only) Makes a Simulator which accepts a context via shared
279-
// pointer.
278+
// (Internal use only) Emplaces a Simulator which accepts a context via
279+
// shared pointer.
280+
//
281+
// The `self` parameter should point to uninitialized storage of at least
282+
// sizeof(Simulator<T>) bytes. It is the calling code's responsibility to
283+
// eventually destroy the Simulator<T> instance, using (for example)
284+
// std::destroy_at.
280285
//
281286
// The shared pointer signature is useful for implementing pydrake memory
282287
// management, because it permits supplying a custom deleter. The context is
283288
// not *actually* shared. The simulator will modify it at will.
284-
static std::unique_ptr<Simulator<T>> MakeWithSharedContext(
285-
const System<T>& system, std::shared_ptr<Context<T>> context);
289+
static void EmplaceWithSharedContext(Simulator<T>* self,
290+
const System<T>& system,
291+
std::shared_ptr<Context<T>> context);
286292
#endif
287293

288294
// TODO(sherm1) Make Initialize() attempt to satisfy constraints.

systems/analysis/test/simulator_test.cc

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "drake/common/autodiff.h"
1919
#include "drake/common/drake_assert.h"
2020
#include "drake/common/drake_copyable.h"
21+
#include "drake/common/scope_exit.h"
2122
#include "drake/common/test_utilities/expect_throws_message.h"
2223
#include "drake/common/test_utilities/is_dynamic_castable.h"
2324
#include "drake/common/text_logging.h"
@@ -814,9 +815,9 @@ GTEST_TEST(SimulatorTest, SecondConstructor) {
814815
EXPECT_EQ(simulator.get_context().get_time(), 3.0);
815816
}
816817

817-
// Tests the internal use (for Python) factory method that takes the context via
818-
// shared pointer.
819-
GTEST_TEST(SimulatorTest, SharedContextFactoryMethod) {
818+
// Tests the internal use (for Python) in-place construction factory method
819+
// that takes the context via shared pointer.
820+
GTEST_TEST(SimulatorTest, SharedContextEmplacementFactoryMethod) {
820821
// Create the spring-mass system and context.
821822
analysis_test::MySpringMassSystem<double> spring_mass(1.0, 1.0, 0.0);
822823
auto context = spring_mass.CreateDefaultContext();
@@ -825,9 +826,15 @@ GTEST_TEST(SimulatorTest, SharedContextFactoryMethod) {
825826
context->SetTime(7.0);
826827
std::shared_ptr<Context<double>> shared_context(std::move(context));
827828

828-
// Construct the simulator with the created context.
829-
auto simulator = Simulator<double>::MakeWithSharedContext(
830-
spring_mass, std::move(shared_context));
829+
// Construct the simulator via placement new with the created context.
830+
std::array<char, sizeof(Simulator<double>)> storage;
831+
Simulator<double>* simulator =
832+
reinterpret_cast<Simulator<double>*>(storage.data());
833+
ScopeExit guard([&]() {
834+
std::destroy_at(simulator);
835+
});
836+
Simulator<double>::EmplaceWithSharedContext(simulator, spring_mass,
837+
std::move(shared_context));
831838

832839
// Verify that context values are equivalent.
833840
EXPECT_EQ(simulator->get_context().get_time(), 7.0);

0 commit comments

Comments
 (0)