pydrake lost reset_integrator

[jwnimmer-tri]

First problem

PR #12520 deprecated the ability to change integrators from pydrake, while printing a misleading message in the meantime:

$ bazel test //bindings/pydrake/systems:py/general_test --nocache_test_results --test_output=streamed

.......drake/bindings/pydrake/systems/test/general_test.py:516: DrakeDeprecationWarning: (Deprecated.)

Deprecated:
    Use void or max-step-size version of reset_integrator() instead.
    This will be removed from Drake on or after 2020-05-01.
  context=simulator.get_mutable_context()))
drake/bindings/pydrake/systems/test/general_test.py:521: DrakeDeprecationWarning: (Deprecated.)

Deprecated:
    Use void or max-step-size version of reset_integrator() instead.
    This will be removed from Drake on or after 2020-05-01.

.....
----------------------------------------------------------------------
Ran 18 tests in 0.009s

It asks me to use a method that does not exist within pydrake. The only bound one is taking a unique_ptr (i.e., the one the test is already calling but receiving a warning about).

\CC @edrumwri @SeanCurtis-TRI @EricCousineau-TRI as author / reviewers of that PR.

In fact, I don't even know how a method that requires a non-inferrable template argument is supposed to work in pydrake? I don't think the current non-deprecated methods are a sufficient API for pydrake.

Second problem

Why is the unit test still passing? I thought that deprecation messages were supposed to be errors by default, unless guarded by catch_drake_warnings.

[jwnimmer-tri]

(2) The reason we lost the warning -> error promotion is because python3 unittest added the following new semantics:

        if warnings is None and not sys.warnoptions:
            # even if DeprecationWarnings are ignored by default
            # print them anyway unless other warnings settings are
            # specified by the warnings arg or the -W python flag
            self.warnings = 'default'

Because we apply our warning filters prior to calling unittest.main(), the unittest filter for any and all Warning instances trumps our more custom filters. Fix pending.

[jwnimmer-tri]

(1) I think the best answer is to re-implement reset_integrator inside pydrake, instead of binding it, as in:

simulator.reset_integrator(RungeKutta2Integrator, 0.1)
simulator.reset_integrator(RungeKutta3Integrator)

So we pass the class (not instance!) and optionally max_step_size. In the pydrake method body we dynamically construct a correctly-initialized IntegratorBase and then move it into the simulator using a drake-internal API taking unique_ptr.

[EricCousineau-TRI]

The best / simplest answer by far (assuming minimal performance + API impact) is to use shared_ptr as you have in your WIP branch mentioned in #12811.

I think after that, perhaps trying to fix the pybind bug itself that I introduced would be best, rather than trying to implement reset_integrator<IntegratorClass>(integrator_args...) in Python. I can look into that later today.

[jwnimmer-tri]

I don't understand. (Did you post in the wrong issue?) The Simulator <-> IntegratorBase is a cyclic relationship that requires careful construction. It seems like we need to keep the construction process sealed to ensure it happens correctly, which means passing the classname to a helper, instead of an already-constructed IntegratorBase.

[EricCousineau-TRI]

Oh... yup, I misunderstood. I was thinking of the old implementation of reset_integrator, and didn't pay attention to the updated semantics. D'oh! So yeah, your suggestion in (1) makes the most sense for parity btw Python and C++. Sorry about that!