Three small problems in Actuator.__init__ validation, all in rocketpy/rocket/actuator/actuator.py. I ran into them while building integration tests for the actuators from BalloonPoppingChallenge.
demand_rate=None is documented but raises
The docstring describes None as the continuous-time actuator, but the check is written as:
assert demand_rate > 0 or demand_rate is None, (
"demand_rate must be positive or None."
)Python evaluates the left operand first, so None > 0 raises before the is None branch is ever reached:
TypeError: '>' not supported between instances of 'NoneType' and 'int'
So the continuous-time mode cannot be constructed at all. Swapping the operands would fix the immediate error:
assert demand_rate is None or demand_rate > 0
The validation disappears under python -O
demand_rate, the actuator range, the rate limit and the time constant are all validated with bare assert, and the interpreter drops assert statements in optimized mode. A negative time constant or rate limit would then be accepted silently. These read like ordinary argument validation rather than internal invariants, so raising would keep them working:
if demand_rate is not None and demand_rate <= 0:
raise ValueError("demand_rate must be positive or None")The initial output is not checked against the range
actuator_initial_output is assigned directly, so an actuator can start outside its own range:
>>> a = ThrottleActuator(throttle_range=(0, 1), initial_throttle=2.0)
>>> a.actuator_output
2.0
The setter clamps later, but _reset() restores the same out-of-range value, so a reset actuator is back outside its range. Worth deciding whether an out-of-range initial value should clamp or raise, and applying that in the constructor.
Happy to send a PR for any of these if the direction looks right. The first two are a couple of lines; the third needs a call on clamp versus raise.
Three small problems in
Actuator.__init__validation, all inrocketpy/rocket/actuator/actuator.py. I ran into them while building integration tests for the actuators from BalloonPoppingChallenge.demand_rate=Noneis documented but raisesThe docstring describes
Noneas the continuous-time actuator, but the check is written as:Python evaluates the left operand first, so
None > 0raises before theis Nonebranch is ever reached:So the continuous-time mode cannot be constructed at all. Swapping the operands would fix the immediate error:
The validation disappears under
python -Odemand_rate, the actuator range, the rate limit and the time constant are all validated with bareassert, and the interpreter drops assert statements in optimized mode. A negative time constant or rate limit would then be accepted silently. These read like ordinary argument validation rather than internal invariants, so raising would keep them working:The initial output is not checked against the range
actuator_initial_outputis assigned directly, so an actuator can start outside its own range:The setter clamps later, but
_reset()restores the same out-of-range value, so a reset actuator is back outside its range. Worth deciding whether an out-of-range initial value should clamp or raise, and applying that in the constructor.Happy to send a PR for any of these if the direction looks right. The first two are a couple of lines; the third needs a call on clamp versus raise.