fix: validation stopped at the arguments, and three paths went around it

The checks added here guard what is passed in. Three things the actuator then
derives from those arguments were left unguarded, and each one puts a
non-finite number into the output the integrator reads.

The range was checked for ordering only, so `(inf, inf)` passed. Clamping a
finite initial output against it stores `inf`, and the next ordinary command
lands on `(1 - alpha) * inf`, which is `0.0 * inf`, and stores NaN. `_reset()`
then restores it at the start of every later flight. `(-inf, -inf)` is the same
in the other direction, and both are reachable through
`Rocket.add_throttle_control`.

Endpoints still are not required to be finite, because an infinite bound is how
this class says "unbounded on that side" and the base default really is
`(-inf, inf)`. The weaker property is what matters and is what is now checked:
clamping a finite value has to give back a finite one. Given `lower <= upper`
that fails exactly when the lower bound is `+inf` or the upper is `-inf`, so
one-sided ranges such as `(0, inf)` are unaffected.

The endpoints are also copied into a new tuple. They were the caller's own
object, so a list mutated after construction moved the range out from under a
validation that had already run.

The IIR coefficient forms `Ts = 1 / demand_rate` before combining it with the
time constant, and that intermediate is not something either argument contains.
A subnormal demand rate of 5e-324 is finite and positive and passes every check
at the boundary, but `1.0 / 5e-324` is `inf` and `inf / (1.0 + inf)` is NaN, so
the first finite command stores NaN and stays there. A time constant of 1e308
overflows the denominator the other way and pins alpha to zero, freezing the
actuator where the answer is about 0.5. Writing the same expression as
`1 / (1 + tau * rate)` avoids forming the intermediate at all. Over 2000 random
pairs across the ranges a real configuration uses, the two agree to 2.2e-16.

The third is in `rocket.py`. Each `add_*_control` built the actuator and the
controller from the same argument, and only the actuator normalized it. With
`sampling_rate="100"` the actuator held `100.0` and the controller held
`"100"`, the call returned a rocket that looked complete, and the failure
surfaced later inside Flight at `1 / controller.sampling_rate`. The controller
now takes the actuator's normalized value. Refusing strings and bools outright
is the other available fix and is a wider behaviour change than this needs.

Thrust vector control reads that value off its x axis: `ThrustVectorActuator2D`
holds no `demand_rate` of its own, only the two axes it builds from the
argument. Its class docstring lists one, along with six other attributes it
also never assigns. That gap is left alone here.

Seven mutations, one per fix and one per call site, each fail a named test; a
control mutation survives. Existing suites unchanged: unit 1945 passed with the
four pre-existing test_sensitivity import failures, integration and acceptance
168 passed.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

Author: thc1006

CHANGELOG.md

@@ -36,6 +36,9 @@ Attention: The newest changes should be on top -->
 
 ### Fixed
 
+- BUG: actuator argument validation reached the output through a range, a
+  derived filter coefficient and a controller [#19](https://github.com/ARRC-Rocket/ActiveRocketPy/pull/19)
+
 ## [v1.13.0] - 2026-07-21
 
 ### Added

rocketpy/rocket/actuator/actuator.py

@@ -47,6 +47,49 @@ def _non_negative_or_none(value, description):
     return number
 
 
+def _range_or_raise(value, description):
+    """Return the two endpoints as a tuple, refusing a range that cannot clamp.
+
+    Endpoints are not held to ``_finite_or_raise``. An infinite bound is how
+    this class says "unbounded on that side", and the base default really is
+    ``(-inf, inf)``. What has to hold instead is weaker and is the property the
+    range is used for: clamping a finite value against it has to give back a
+    finite value.
+
+    That fails in exactly two cases, and ordering makes them one. Given
+    ``lower <= upper``, a lower bound of ``+inf`` forces the upper bound to
+    match, and an upper bound of ``-inf`` forces the lower bound to match; in
+    both, ``np.clip`` returns the infinity. So ``(inf, inf)`` passed the
+    ordering check, took a finite initial output of 0.5, and stored ``inf``,
+    after which the first ordinary command landed on
+    ``(1 - alpha) * inf == 0.0 * inf`` and stored NaN, which ``_reset()`` then
+    restored on every subsequent flight. A one-sided bound such as ``(0, inf)``
+    is unaffected and stays allowed.
+
+    The endpoints are also copied into a new tuple. They were stored as the
+    caller's own object, so a list mutated after construction moved the range
+    out from under a validation that had already run.
+    """
+    try:
+        lower, upper = value
+    except (TypeError, ValueError) as error:
+        raise ValueError(f"{description} must be a pair of numbers.") from error
+    try:
+        lower = float(lower)
+        upper = float(upper)
+    except (TypeError, ValueError) as error:
+        raise ValueError(f"{description} endpoints must be numbers.") from error
+    # Written as a positive test because NaN fails every ordered comparison, so
+    # a range of two NaNs would slip through a check phrased as a negation.
+    if not lower <= upper:
+        raise ValueError(f"{description}[0] must be <= {description}[1].")
+    if lower == np.inf or upper == -np.inf:
+        raise ValueError(
+            f"{description} {(lower, upper)} cannot clamp anything to a finite value."
+        )
+    return (lower, upper)
+
+
 class Actuator(ABC):
     """Abstract class used to define actuators.
 
@@ -102,12 +145,12 @@ def __init__(
         # zero and freezes the output.
         #
         # The range endpoints are deliberately not put through this. The base
-        # default really is (-inf, inf), meaning an actuator with no range.
+        # default really is (-inf, inf), meaning an actuator with no range, so
+        # they get the weaker check in _range_or_raise instead.
         self.demand_rate = _positive_or_none(demand_rate, "demand_rate")
 
-        if not actuator_range[0] <= actuator_range[1]:
-            raise ValueError("actuator_range[0] must be <= actuator_range[1].")
-        self.actuator_range = actuator_range
+        self.actuator_range = _range_or_raise(actuator_range, "actuator_range")
+        actuator_range = self.actuator_range
 
         self.actuator_rate_limit = _non_negative_or_none(
             actuator_rate_limit, "actuator_rate_limit"
@@ -153,9 +196,19 @@ def _update_iir_coefficients(self):
 
         if self.actuator_time_constant is not None and self.actuator_time_constant > 0:
             if self.demand_rate is not None:
-                demand_period = 1.0 / self.demand_rate
-                self._alpha = demand_period / (
-                    self.actuator_time_constant + demand_period
+                # Algebraically Ts / (tau + Ts) with Ts = 1 / demand_rate, but
+                # written without forming Ts. That intermediate overflows for a
+                # subnormal demand rate: 1.0 / 5e-324 is inf, inf / (1.0 + inf)
+                # is NaN, and the filter then stores NaN out of a command that
+                # passed every check on the way in. At the other end a huge time
+                # constant makes the denominator overflow and pins alpha to
+                # zero, freezing the actuator, where 1e-308 with 1e308 should
+                # give about 0.5. Neither shape is reachable from a physical
+                # configuration, and neither costs anything to rule out: over
+                # 2000 random pairs across the ranges that are, the two forms
+                # agree to 2.2e-16.
+                self._alpha = 1.0 / (
+                    1.0 + self.actuator_time_constant * self.demand_rate
                 )
             else:
                 warnings.warn(

rocketpy/rocket/rocket.py

@@ -2095,7 +2095,21 @@ def add_thrust_vector_control(
         _controller = _Controller(
             interactive_objects=thrust_vector_control,
             controller_function=controller_function,
-            sampling_rate=sampling_rate,
+            # The actuator's normalized rate, not the argument. The actuator
+            # runs its own validation and stores a float, so handing the
+            # controller the original leaves the two holding different types
+            # for one quantity: sampling_rate="100" gives the actuator 100.0
+            # and the controller "100", this call returns successfully, and the
+            # failure surfaces later in Flight at 1 / controller.sampling_rate,
+            # by which point the rocket is already half built.
+            #
+            # Read off the x axis because ThrustVectorActuator2D holds no
+            # demand_rate of its own. Its class docstring lists one, along with
+            # six other attributes it also never assigns, but __init__ only
+            # forwards the argument to the two axes. Both are built from that
+            # one argument, so either axis gives the same number, and reaching
+            # into .x is what Flight already does to reset this class.
+            sampling_rate=thrust_vector_control.x.demand_rate,
             initial_observed_variables=initial_observed_variables,
             name=controller_name,
         )
@@ -2228,7 +2242,8 @@ def add_roll_control(
         _controller = _Controller(
             interactive_objects=roll_control,
             controller_function=controller_function,
-            sampling_rate=sampling_rate,
+            # The actuator's normalized rate. See add_thrust_vector_control.
+            sampling_rate=roll_control.demand_rate,
             initial_observed_variables=initial_observed_variables,
             name=controller_name,
         )
@@ -2364,7 +2379,8 @@ def add_throttle_control(
         _controller = _Controller(
             interactive_objects=throttle_control,
             controller_function=controller_function,
-            sampling_rate=sampling_rate,
+            # The actuator's normalized rate. See add_thrust_vector_control.
+            sampling_rate=throttle_control.demand_rate,
             initial_observed_variables=initial_observed_variables,
             name=controller_name,
         )

tests/unit/rocket/test_actuators.py

@@ -693,3 +693,182 @@ def test_clamping_applied(self):
         actuator.actuator_output = 15.0
         # Output should be clamped to range
         assert actuator.roll_torque == 10.0
+
+
+class TestARangeThatCannotClampIsRefused:
+    """The range was checked for ordering only, and stored by reference.
+
+    Both halves let a finite value become a non-finite stored output, which is
+    the one thing the validation around it exists to prevent. Measured before
+    the fix: ``(inf, inf)`` passed ``lower <= upper``, ``np.clip(0.5, inf, inf)``
+    stored ``inf``, and the next ordinary command reached
+    ``(1 - alpha) * inf``, which is ``0.0 * inf``, and stored NaN. ``_reset()``
+    put it back at the start of every later flight.
+    """
+
+    @pytest.mark.parametrize("limits", [(math.inf, math.inf), (-math.inf, -math.inf)])
+    def test_a_range_with_nothing_finite_in_it_is_refused(self, limits):
+        with pytest.raises(ValueError, match="clamp"):
+            ThrottleActuator(throttle_range=limits, initial_throttle=0.5)
+
+    def test_a_range_of_nans_is_refused(self):
+        """NaN fails every ordered comparison, so the ordering check catches it
+        only because that check is written as a positive test."""
+        with pytest.raises(ValueError):
+            ThrottleActuator(throttle_range=(math.nan, math.nan))
+
+    @pytest.mark.parametrize("limits", [(0.0,), (0.0, 1.0, 2.0), 1.0])
+    def test_a_range_that_is_not_a_pair_is_refused(self, limits):
+        with pytest.raises(ValueError):
+            ThrottleActuator(throttle_range=limits)
+
+    @pytest.mark.parametrize(
+        "limits",
+        [(-math.inf, math.inf), (0.0, math.inf), (-math.inf, 1.0), (0.0, 1.0)],
+    )
+    def test_a_range_that_can_clamp_still_builds(self, limits):
+        """The half that stops this being satisfied by refusing everything.
+
+        An infinite bound is how this class says "unbounded on that side", and
+        the base default really is ``(-inf, inf)``, so only a range with no
+        finite value on the clamping side may be refused.
+        """
+        actuator = ThrottleActuator(throttle_range=limits, initial_throttle=0.5)
+        actuator.throttle = 0.25
+
+        assert math.isfinite(actuator.throttle)
+
+    def test_mutating_the_range_passed_in_does_not_move_the_actuator(self):
+        """It was the caller's own list, so a validated invariant could be
+        edited away after the fact."""
+        limits = [0.0, 1.0]
+        actuator = ThrottleActuator(throttle_range=limits, initial_throttle=0.5)
+
+        limits[:] = [math.inf, math.inf]
+        actuator.throttle = 0.25
+
+        assert actuator.actuator_range == (0.0, 1.0)
+        assert actuator.throttle == 0.25
+
+
+class TestTheFilterCoefficientStaysFinite:
+    """``alpha`` is derived, and validating only the inputs left it unchecked.
+
+    ``Ts / (tau + Ts)`` with ``Ts = 1 / demand_rate`` forms an intermediate that
+    the arguments themselves never contain. Both arguments below are finite and
+    pass every check at the boundary.
+    """
+
+    def test_a_subnormal_demand_rate_does_not_give_a_nan_coefficient(self):
+        """Measured before the fix: ``1.0 / 5e-324`` is inf, ``inf / (1.0 + inf)``
+        is NaN, and the first finite command then stored NaN and stayed there."""
+        actuator = ThrottleActuator(
+            demand_rate=5e-324,
+            throttle_time_constant=1.0,
+            throttle_range=(0.0, 1.0),
+            initial_throttle=0.5,
+        )
+        actuator.throttle = 0.25
+
+        assert math.isfinite(actuator._alpha)
+        assert math.isfinite(actuator.throttle)
+
+    def test_a_huge_time_constant_does_not_pin_the_coefficient_to_zero(self):
+        """The other end of the same overflow. The denominator went infinite and
+        alpha came out 0, which freezes the actuator at its initial value; the
+        answer here is about 0.5."""
+        actuator = ThrottleActuator(
+            demand_rate=1e-308,
+            throttle_time_constant=1e308,
+            throttle_range=(0.0, 1.0),
+            initial_throttle=0.5,
+        )
+
+        assert actuator._alpha == pytest.approx(0.5)
+
+    @pytest.mark.parametrize(
+        "demand_rate, time_constant",
+        [(1e-3, 1e-6), (1.0, 1.0), (100.0, 0.05), (1e4, 1e3)],
+    )
+    def test_the_two_forms_agree_where_both_work(self, demand_rate, time_constant):
+        """So the rewrite is a rewrite and not a change of behaviour. Measured
+        over 2000 random pairs across these ranges: the largest difference is
+        2.2e-16."""
+        actuator = ThrottleActuator(
+            demand_rate=demand_rate,
+            throttle_time_constant=time_constant,
+            throttle_range=(0.0, 1.0),
+        )
+        demand_period = 1.0 / demand_rate
+
+        assert actuator._alpha == pytest.approx(
+            demand_period / (time_constant + demand_period), rel=1e-12
+        )
+
+
+def _no_op_controller(time, sampling_rate, state, state_history, observed, interactive):  # pylint: disable=unused-argument
+    """A controller that commands nothing, so these tests are about the wiring."""
+    return None
+
+
+class TestTheControllerAndTheActuatorShareOneSamplingRate:
+    """``add_*_control`` built the two halves from the same argument.
+
+    The actuator normalizes what it is given and stores a float. The controller
+    was handed the original object, so one quantity ended up held twice, in two
+    types. Nothing complained: the call returned a rocket that looked complete,
+    and the failure surfaced later inside Flight at
+    ``controller_time_step = 1 / controller.sampling_rate``, by which point the
+    actuator and the controller were already attached.
+
+    Passing the actuator's normalized value is the smaller of the two available
+    fixes. Refusing strings and bools outright is the other, and it is a wider
+    behaviour change than this needs.
+    """
+
+    # The third entry reaches through .x because ThrustVectorActuator2D keeps
+    # no demand_rate of its own, only the two axes it builds from the argument.
+    ADDERS = [
+        ("add_roll_control", "roll_control", {"max_roll_torque": 10.0}, False),
+        ("add_throttle_control", "throttle_control", {}, False),
+        (
+            "add_thrust_vector_control",
+            "thrust_vector_control",
+            {"max_gimbal_angle": 5.0},
+            True,
+        ),
+    ]
+
+    @staticmethod
+    def _rate_of(rocket, attribute, per_axis):
+        actuator = getattr(rocket, attribute)
+        return (actuator.x if per_axis else actuator).demand_rate
+
+    @pytest.mark.parametrize("adder, attribute, extra, per_axis", ADDERS)
+    @pytest.mark.parametrize("given", ["100", True, 100])
+    def test_both_halves_hold_the_same_normalized_value(
+        self, calisto, adder, attribute, extra, per_axis, given
+    ):
+        """``True`` is in here because ``float(True)`` is 1.0, so a bool reaches
+        the actuator as a rate and used to reach the controller as a bool."""
+        getattr(calisto, adder)(
+            controller_function=_no_op_controller, sampling_rate=given, **extra
+        )
+        rate = self._rate_of(calisto, attribute, per_axis)
+        controller = calisto._controllers[-1]
+
+        assert controller.sampling_rate == rate
+        assert type(controller.sampling_rate) is type(rate)
+
+    @pytest.mark.parametrize(
+        "adder, extra", [(adder, extra) for adder, _, extra, _ in ADDERS]
+    )
+    def test_the_controller_rate_is_usable_as_a_time_step(self, calisto, adder, extra):
+        """The exact expression Flight evaluates, which is where a string blew
+        up with a TypeError after the rocket had already been assembled."""
+        getattr(calisto, adder)(
+            controller_function=_no_op_controller, sampling_rate="100", **extra
+        )
+        controller = calisto._controllers[-1]
+
+        assert 1 / controller.sampling_rate == pytest.approx(0.01)