Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Attention: The newest changes should be on top -->

### Fixed

- BUG: Accept the callable parachute triggers `StochasticParachute` documents, and reject the ones it cannot mean. An invalid string, an empty list or a boolean now fails during validation instead of reaching `Parachute` or becoming a one-metre height trigger. [#1103](https://github.com/RocketPy-Team/RocketPy/pull/1103)
- BUG: rocket with a late-starting thrust curve never leaves the rail [#1085](https://github.com/RocketPy-Team/RocketPy/pull/1085)

## [v1.13.0] - 2026-07-21
Expand Down
43 changes: 34 additions & 9 deletions rocketpy/stochastic/stochastic_parachute.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,21 @@
from .stochastic_model import StochasticModel


def _is_a_trigger(member):
"""One of the three forms ``Parachute`` accepts, and no more.

``(int, float)`` deliberately, matching ``Parachute``'s own check rather
than ``numbers.Real``: that would take ``numpy.int64``, which ``Parachute``
refuses, so widening here only moves the failure to create time. ``bool``
is excluded because it is an ``int``, and would arrive as a height of one.
"""
if callable(member):
return True
if isinstance(member, str):
return member.lower() == "apogee"
return isinstance(member, (int, float)) and not isinstance(member, bool)


class StochasticParachute(StochasticModel):
"""A Stochastic Parachute class that inherits from StochasticModel.

Expand Down Expand Up @@ -114,16 +129,26 @@ def __init__(
)

def _validate_trigger(self, trigger):
"""Validates the trigger input. If the trigger input argument is not
None, it must be:
- a list of callables, string "apogee" or ints/floats
- a tuple that will be further validated in the StochasticModel class
"""Validates the trigger input. If not None, it must be a non-empty
list whose members are each a callable, the string "apogee", or a
height. One of those is chosen per simulation.
"""
if trigger is not None:
assert isinstance(trigger, list) and all(
isinstance(member, (str, int, float) or callable(member))
for member in trigger
), "`trigger` must be a list of callables, string 'apogee' or ints/floats"
if trigger is None:
return

valid = (
isinstance(trigger, list)
and bool(trigger)
and all(_is_a_trigger(member) for member in trigger)
)
# Raised rather than asserted: `python -O` strips an assert, and this
# is the only thing standing between a bad trigger and a Parachute
# that either refuses it much later or reads True as a height of 1.
if not valid:
raise AssertionError(
"`trigger` must be a non-empty list whose members are "
"callables, the string 'apogee', or heights"
)

def _validate_noise(self, noise):
"""Validates the noise input. If the noise input argument is not
Expand Down
115 changes: 115 additions & 0 deletions tests/unit/stochastic/test_stochastic_parachute.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import inspect

import numpy as np
import pytest

from rocketpy.stochastic import StochasticParachute
from rocketpy.rocket.parachute import Parachute


Expand All @@ -19,3 +25,112 @@ class creates a StochasticParachute object from the randomly generated
"""
obj = stochastic_main_parachute.create_object()
assert isinstance(obj, Parachute)


def _at_apogee(pressure, height, state): # pylint: disable=unused-argument
"""A trigger of the kind `Parachute` and `Flight` already accept.

Keeps the full signature rather than underscoring the unused two, since the
signature is the contract being tested."""
return state[5] < 0


@pytest.mark.parametrize(
"trigger",
[[_at_apogee], ["apogee"], [800], [_at_apogee, "apogee", 800]],
ids=["callable", "apogee", "height", "mixed"],
)
def test_every_documented_trigger_form_is_accepted(calisto_main_chute, trigger):
"""The docstring promises callables, "apogee" and numbers. The check read
`isinstance(member, (str, int, float) or callable(member))`, and a non-empty
type tuple is truthy, so the `or` short-circuited and callables were
refused. The two non-callable forms passed throughout, which is why it went
unnoticed."""
StochasticParachute(calisto_main_chute, trigger=trigger)


@pytest.mark.parametrize(
"trigger",
[
_at_apogee,
"apogee",
800,
(800,),
[],
[None],
[{}],
["banana"],
[True],
[_at_apogee, None],
],
ids=str,
)
def test_a_trigger_that_is_not_a_list_of_those_is_refused(calisto_main_chute, trigger):
"""The control, and four that the check used to wave through.

`Parachute` refuses "banana" with a ValueError, so accepting it here only
moved the failure to create time. `True` is worse: it is an `int`, so it
was taken as a height of one metre. An empty list passed because `all([])`
is True. And the docstring's tuple form was never implemented.
"""
with pytest.raises(AssertionError, match="must be a non-empty list"):
StochasticParachute(calisto_main_chute, trigger=trigger)


@pytest.mark.parametrize(
"member",
[_at_apogee, "apogee", "APOGEE", 800, 800.0, np.float64(800)],
ids=str,
)
def test_what_this_accepts_is_what_a_parachute_accepts(calisto_main_chute, member):
"""The property, rather than a list of types. Anything this lets through
has to survive `Parachute`, or the check has only moved the failure."""
StochasticParachute(calisto_main_chute, trigger=[member])

Parachute("probe", 10.0, member, 105, 1.5)


@pytest.mark.parametrize("member", [np.int64(800), np.int32(800)], ids=str)
def test_a_numpy_integer_is_refused_here_because_parachute_refuses_it(
calisto_main_chute, member
):
"""`Parachute` checks `isinstance(trigger, (int, float))`. `numpy.float64`
subclasses `float` and passes; `numpy.int64` subclasses neither and raises.

So this check matches that one rather than `numbers.Real`, which would be
the wider and more natural spelling but would let these through to fail at
create time. The asymmetry is `Parachute`'s and is worth fixing there.
"""
with pytest.raises(ValueError, match="Unable to set the trigger"):
Parachute("probe", 10.0, member, 105, 1.5)

with pytest.raises(AssertionError, match="must be a non-empty list"):
StochasticParachute(calisto_main_chute, trigger=[member])


def test_the_check_is_not_stripped_by_python_dash_o():
"""`python -O` removes an `assert` outright, and this check is the only
thing between a bad trigger and a `Parachute` that either refuses it much
later or reads `True` as a height."""
source = inspect.getsource(StochasticParachute._validate_trigger)

assert "raise AssertionError" in source
assert not any(line.strip().startswith("assert ") for line in source.splitlines())


def test_a_callable_trigger_reaches_the_parachute_and_gets_called(
calisto_main_chute,
):
"""Constructing the wrapper is not the property that matters. The callable
has to survive `create_object` and be what `Flight` ends up calling."""
stochastic = StochasticParachute(calisto_main_chute, trigger=[_at_apogee])
stochastic._set_stochastic(42)

built = stochastic.create_object()

assert built.trigger is _at_apogee
# 13, matching the state Flight passes: x y z vx vy vz e0 e1 e2 e3 wx wy wz
descending = [0.0] * 5 + [-5.0] + [0.0] * 7
ascending = [0.0] * 5 + [5.0] + [0.0] * 7
assert built.triggerfunc(0.0, 100.0, descending, [], [])
assert not built.triggerfunc(0.0, 100.0, ascending, [], [])
Loading