Skip to content

Commit f5028d0

Browse files
Malmahrouqi3Gui-FernandesBRCopilotclaude
authored
ENH: Discrete and Continuous Controllers (#946)
* ENH: added discrte and continuous controller functions * linted rocketpy/rocket/rocket.py * lint fix return None to return self in discrete controller function * BUG: fixed cont ctrl calls integrated into the ODE solver * lint fixes * lint fix: removed return None * implemented copilot suggestions * lint fix, extra spaces * ENH: merged dis+con and added None for sampling rate of cont * BUG: copilot comments addressed * ENH: finalize continuous controller handling Optimize continuous controller state-history handling to avoid repeated list rebuilding, align controller sampling-rate docs with None-based continuous mode, and add regression coverage to ensure continuous controllers do not create time nodes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * DOC: update changelog for PR 946 Add unreleased changelog entry for Discrete and Continuous Controllers in PR #946. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * BUG: pass self.solution to continuous controllers for parity with discrete The continuous-controller path fed the controller a time-stripped `_controller_state_history` (rows `[x, y, z, vx, vy, vz, ...]`), while the discrete path (__process_sensors_and_controllers_at_current_node) passes `self.solution`, whose rows are time-prefixed (`[t, x, y, z, vx, vy, vz, ...]`). A single controller function therefore received two different state_history layouts depending on discrete vs continuous mode: index [5] was vy for discrete but vz for continuous. Controllers written/tuned against the discrete layout (e.g. the airbrakes example) misbehaved under continuous control -- reading vz where they expected vy, deployment logic exploded and apogee was crushed. Pass `self.solution` directly, matching the discrete path exactly. This makes the two paths behaviorally identical, removes the parallel _controller_state_history list (and its per-step append / initialization), and is O(1) per step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * MNT: add _Controller.is_continuous and fix prints for continuous controllers - Add a single `_Controller.is_continuous` property (sampling_rate is None) and use it as the one source of truth for the continuous/discrete dispatch in Flight (__init_controllers and time-node creation), replacing two duplicated `sampling_rate is None` checks. - Fix _ControllerPrints.controller_function(): `f"{sampling_rate:.3f}"` raised TypeError for continuous controllers (sampling_rate=None). Print "continuous (every solver step)" instead. - __str__ now reads "with continuous sampling." instead of "... None Hz." Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * DOC: correct state_history contract for controller functions The state_history handed to controller functions is `Flight.solution`, whose rows are TIME-PREFIXED `[t, x, y, z, vx, vy, vz, ...]` -- one leading time element ahead of the `state` layout, and the last row is the most recent recorded step (not the "previous" state). The docstrings previously described the rows as bare state vectors `[x, y, z, ...]` with `[-1]` as the previous state, an off-by-one-column contract that silently breaks controllers (this is what caused the airbrakes continuous-mode misbehavior). Also document that `sampling_rate` is None for continuous controllers. Updated in _Controller.__init__, _Controller.__call__, and Rocket.add_air_brakes controller docstrings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * TST: cover continuous controller per-step invocation and state_history layout Adds an integration test that runs a real flight with a continuous air-brakes controller (sampling_rate=None) and asserts it is invoked on every solver step (>50 calls), always receives sampling_rate=None, and receives time-prefixed state_history rows (len == len(state)+1) -- the same layout as the discrete path. Previously only node-skipping was covered; per-step invocation was not. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * MNT: satisfy ruff format and pylint in CI lint job - flight.py: collapse the _continuous_controllers comprehension to one line (ruff format --check). - test_flight.py: drop the useless `return None` at the end of the recording controller (pylint R1711). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gui-FernandesBR <guilherme_fernandes@usp.br> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b3ec999 commit f5028d0

7 files changed

Lines changed: 128 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ Attention: The newest changes should be on top -->
4545
- ENH: Monte Carlo Formatting Options [#947](https://github.com/RocketPy-Team/RocketPy/pull/947)
4646
- ENH: ENH: Auto-Detection of Pressure Conversion Factor [#966](https://github.com/RocketPy-Team/RocketPy/pull/966)
4747
- ENH: Auto-Detection of Pressure Conversion Factor [#966](https://github.com/RocketPy-Team/RocketPy/pull/966)
48+
- ENH: Discrete and Continuous Controllers [#946](https://github.com/RocketPy-Team/RocketPy/pull/946)
4849
- DOC: Add aerodynamic surfaces user guide [#1043](https://github.com/RocketPy-Team/RocketPy/pull/1043)
4950
- ENH: Add RingClusterMotor for annular clustered motor modeling [#924](https://github.com/RocketPy-Team/RocketPy/pull/924)
5051
- ENH: MNT: introduce pressure unit conversion when using forecast/reanalysis/ensemble data [#955](https://github.com/RocketPy-Team/RocketPy/pull/955)

rocketpy/control/controller.py

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def __init__(
1717
self,
1818
interactive_objects,
1919
controller_function,
20-
sampling_rate,
20+
sampling_rate=None,
2121
initial_observed_variables=None,
2222
name="Controller",
2323
):
@@ -38,16 +38,19 @@ def __init__(
3838
This function is expected to take the following arguments, in order:
3939
4040
1. `time` (float): The current simulation time in seconds.
41-
2. `sampling_rate` (float): The rate at which the controller
42-
function is called, measured in Hertz (Hz).
41+
2. `sampling_rate` (float or None): The rate at which the controller
42+
function is called, measured in Hertz (Hz). It is None for
43+
continuous controllers (called every solver step), so any
44+
`1 / sampling_rate` computation must guard against None.
4345
3. `state` (list): The state vector of the simulation, structured as
4446
`[x, y, z, vx, vy, vz, e0, e1, e2, e3, wx, wy, wz]`.
4547
4. `state_history` (list): A record of the rocket's state at each
46-
step throughout the simulation. The state_history is organized as
47-
a list of lists, with each sublist containing a state vector. The
48-
last item in the list always corresponds to the previous state
49-
vector, providing a chronological sequence of the rocket's
50-
evolving states.
48+
step throughout the simulation. It is organized as a list of
49+
lists, ordered oldest to newest, where each sublist is a
50+
*time-prefixed* state row `[t, x, y, z, vx, vy, vz, e0, e1, e2,
51+
e3, wx, wy, wz]` (i.e. the same layout as `Flight.solution`, one
52+
leading `time` element ahead of the `state` layout in item 3).
53+
The last item corresponds to the most recent recorded step.
5154
5. `observed_variables` (list): A list containing the variables that
5255
the controller function returns. The return of each controller
5356
function call is appended to the observed_variables list. The
@@ -71,12 +74,14 @@ def __init__(
7174
objects as needed. The function return statement can be used to save
7275
relevant information in the `observed_variables` list.
7376
74-
.. note:: The function will be called according to the sampling rate
75-
specified.
76-
sampling_rate : float
77+
.. note:: The function will be called according to the sampling
78+
rate specified. If `sampling_rate` is None, the controller
79+
function is called at every solver step of the simulation.
80+
sampling_rate : float, optional
7781
The sampling rate of the controller function in Hertz (Hz). This
7882
means that the controller function will be called every
79-
`1/sampling_rate` seconds.
83+
`1/sampling_rate` seconds. If None, it is treated as a
84+
continuous controller and called at every solver step.
8085
initial_observed_variables : list, optional
8186
A list of the initial values of the variables that the controller
8287
function returns. This list is used to initialize the
@@ -178,10 +183,12 @@ def __call__(self, time, state_vector, state_history, sensors, environment):
178183
179184
`[x, y, z, vx, vy, vz, e0, e1, e2, e3, wx, wy, wz]`.
180185
state_history : list
181-
A list containing the state history of the simulation. The state
182-
history is a list of every state vector of every step of the
183-
simulation. The state history is a list of lists, where each
184-
sublist is a state vector and is ordered from oldest to newest.
186+
A list containing the state history of the simulation, ordered
187+
oldest to newest. Each sublist is a *time-prefixed* state row
188+
`[t, x, y, z, vx, vy, vz, e0, e1, e2, e3, wx, wy, wz]` (the same
189+
layout as `Flight.solution`, with a leading `time` element ahead of
190+
the `state_vector` layout). The last item is the most recent
191+
recorded step.
185192
sensors : list
186193
A list of sensors that are attached to the rocket. The most recent
187194
measurements of the sensors are provided with the
@@ -208,7 +215,15 @@ def __call__(self, time, state_vector, state_history, sensors, environment):
208215
if observed_variables is not None:
209216
self.observed_variables.append(observed_variables)
210217

218+
@property
219+
def is_continuous(self):
220+
"""bool: True if the controller runs at every solver step (i.e.
221+
``sampling_rate`` is None), False if it is sampled at a fixed rate."""
222+
return self.sampling_rate is None
223+
211224
def __str__(self):
225+
if self.is_continuous:
226+
return f"Controller '{self.name}' with continuous sampling."
212227
return f"Controller '{self.name}' with sampling rate {self.sampling_rate} Hz."
213228

214229
def info(self):

rocketpy/prints/controller_prints.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,10 @@ def controller_function(self):
4141
print(
4242
"Controller function: " + self.controller.controller_function.__name__
4343
)
44-
print(f"Controller refresh rate: {self.controller.sampling_rate:.3f} Hz")
44+
if self.controller.is_continuous:
45+
print("Controller refresh rate: continuous (every solver step)")
46+
else:
47+
print(f"Controller refresh rate: {self.controller.sampling_rate:.3f} Hz")
4548

4649
def interactive_objects(self):
4750
"""Prints interactive objects."""

rocketpy/rocket/rocket.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1826,15 +1826,19 @@ def add_air_brakes(
18261826
This function is expected to take the following arguments, in order:
18271827
18281828
1. `time` (float): The current simulation time in seconds.
1829-
2. `sampling_rate` (float): The rate at which the controller
1830-
function is called, measured in Hertz (Hz).
1829+
2. `sampling_rate` (float or None): The rate at which the controller
1830+
function is called, measured in Hertz (Hz). It is None for
1831+
continuous controllers (called every solver step), so any
1832+
`1 / sampling_rate` computation must guard against None.
18311833
3. `state` (list): The state vector of the simulation, structured as
18321834
`[x, y, z, vx, vy, vz, e0, e1, e2, e3, wx, wy, wz]`.
18331835
4. `state_history` (list): A record of the rocket's state at each
1834-
step throughout the simulation. The state_history is organized as a
1835-
list of lists, with each sublist containing a state vector. The last
1836-
item in the list always corresponds to the previous state vector,
1837-
providing a chronological sequence of the rocket's evolving states.
1836+
step throughout the simulation. It is organized as a list of
1837+
lists, ordered oldest to newest, where each sublist is a
1838+
*time-prefixed* state row `[t, x, y, z, vx, vy, vz, e0, e1, e2,
1839+
e3, wx, wy, wz]` (the same layout as `Flight.solution`, one
1840+
leading `time` element ahead of the `state` layout in item 3).
1841+
The last item corresponds to the most recent recorded step.
18381842
5. `observed_variables` (list): A list containing the variables that
18391843
the controller function returns. The initial value in the first
18401844
step of the simulation of this list is provided by the

rocketpy/simulation/flight.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,14 @@ def __simulate(self, verbose):
774774
print(f"Current Simulation Time: {self.t:3.4f} s", end="\r")
775775
logger.debug("Current Simulation Time: %3.4f s", self.t)
776776

777+
for controller in self._continuous_controllers:
778+
controller(
779+
self.t,
780+
self.y_sol,
781+
self.solution,
782+
self.sensors,
783+
self.env,
784+
)
777785
if self.__check_simulation_events(phase, phase_index, node_index):
778786
break # Stop if simulation termination event occurred
779787

@@ -1585,6 +1593,7 @@ def __init_equations_of_motion(self):
15851593
def __init_controllers(self):
15861594
"""Initialize controllers and sensors"""
15871595
self._controllers = self.rocket._controllers[:]
1596+
self._continuous_controllers = [c for c in self._controllers if c.is_continuous]
15881597
self.sensors = self.rocket.sensors.get_components()
15891598

15901599
# reset controllable object to initial state (only airbrakes for now)
@@ -4468,6 +4477,9 @@ def add_parachutes(self, parachutes, t_init, t_end):
44684477

44694478
def add_controllers(self, controllers, t_init, t_end):
44704479
for controller in controllers:
4480+
# Skip node creation for continuous controllers
4481+
if controller.is_continuous:
4482+
continue
44714483
# Calculate start of sampling time nodes
44724484
controller_time_step = 1 / controller.sampling_rate
44734485
controller_node_list = [

tests/integration/simulation/test_flight.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -873,3 +873,46 @@ def test_environment_methods_accessible_in_controller(
873873

874874
# Verify all environment methods were successfully called
875875
assert all(methods_called.values()), f"Not all methods called: {methods_called}"
876+
877+
878+
def test_continuous_controller_invoked_every_step(calisto_robust, example_plain_env):
879+
"""A continuous controller (sampling_rate=None) must be called on every
880+
solver step and receive the same state_history layout as a discrete one:
881+
time-prefixed rows (`[t, *state]`), one element longer than ``state``.
882+
This locks in the discrete/continuous parity contract."""
883+
calls = {"count": 0, "sampling_rates": set(), "row_len_matches": True}
884+
885+
def recording_controller( # pylint: disable=unused-argument
886+
time, sampling_rate, state, state_history, observed_variables, air_brakes
887+
):
888+
calls["count"] += 1
889+
calls["sampling_rates"].add(sampling_rate)
890+
# state_history rows are time-prefixed: exactly one longer than state
891+
if len(state_history[-1]) != len(state) + 1:
892+
calls["row_len_matches"] = False
893+
894+
calisto_robust.parachutes = []
895+
calisto_robust.add_air_brakes(
896+
drag_coefficient_curve="data/rockets/calisto/air_brakes_cd.csv",
897+
controller_function=recording_controller,
898+
sampling_rate=None, # continuous
899+
clamp=True,
900+
)
901+
902+
flight = Flight(
903+
rocket=calisto_robust,
904+
environment=example_plain_env,
905+
rail_length=5.2,
906+
inclination=85,
907+
heading=0,
908+
time_overshoot=False,
909+
terminate_on_apogee=True,
910+
)
911+
912+
assert flight.t_final > 0
913+
# Called many times (once per solver step), far more than any fixed rate
914+
assert calls["count"] > 50
915+
# The controller always saw sampling_rate=None (continuous)
916+
assert calls["sampling_rates"] == {None}
917+
# And time-prefixed rows, consistent with the discrete controller path
918+
assert calls["row_len_matches"]

tests/unit/simulation/test_flight_time_nodes.py

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
TimeNode.
33
"""
44

5-
# from rocketpy.rocket import Parachute, _Controller
5+
from rocketpy.control.controller import _Controller
66

77

88
def test_time_nodes_init(flight_calisto):
@@ -49,6 +49,32 @@ def test_time_nodes_add_node(flight_calisto):
4949
# TODO: implement this test
5050

5151

52+
def test_time_nodes_add_controllers_skips_continuous_controllers(flight_calisto):
53+
"""Ensure only discrete controllers create time nodes."""
54+
# Arrange
55+
discrete_controller = _Controller(
56+
interactive_objects=[],
57+
controller_function=lambda t, sr, sv, sh, ov, io: None,
58+
sampling_rate=10,
59+
name="Discrete",
60+
)
61+
continuous_controller = _Controller(
62+
interactive_objects=[],
63+
controller_function=lambda t, sr, sv, sh, ov, io: None,
64+
sampling_rate=None,
65+
name="Continuous",
66+
)
67+
time_nodes = flight_calisto.TimeNodes()
68+
69+
# Act
70+
time_nodes.add_controllers([discrete_controller, continuous_controller], 0, 1)
71+
72+
# Assert
73+
assert len(time_nodes) == 11
74+
assert all(node._controllers == [discrete_controller] for node in time_nodes)
75+
assert all(continuous_controller not in node._controllers for node in time_nodes)
76+
77+
5278
def test_time_nodes_sort(flight_calisto):
5379
time_nodes = flight_calisto.TimeNodes()
5480
time_nodes.add_node(3.0, [], [], [])

0 commit comments

Comments
 (0)