ENH: seed sensor measurement noise per instance#1052
Conversation
Sensor noise (Accelerometer, Gyroscope, Barometer, GnssReceiver) was drawn from the process-global NumPy RNG, so it could not be seeded or reproduced, and it was unsafe under parallel or forked execution where workers share or reset the global RNG (issue RocketPy-Team#1042). Thread an optional seed argument through the Sensor base classes. Each sensor now owns a numpy.random.Generator from np.random.default_rng(seed) and draws its white noise and random walk from it instead of np.random. seed=None keeps the noise random but per-instance, so existing behaviour is unchanged unless a seed is given. Also adds tests/unit/sensors/test_sensor_seeding.py covering reproducibility, decorrelation across seeds, independence from the global RNG, and the GnssReceiver path. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Sensor noise now comes from each sensor's own Generator, so the autouse `_seed_rng` fixture that seeded the global `np.random` no longer made the noisy assertions deterministic; they would flake on CI. Pass `seed=42` to the noisy accelerometer, gyroscope, barometer, and gnss fixtures so their noise is reproducible through the new per-instance seeding, and drop the now-defunct `_seed_rng` fixture. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
There was a problem hiding this comment.
I kept
seedout of the docstrings andto_dict/from_dictto keep the change minimal and consistent with the existing style (a couple of the sensor__init__s don't document every parameter either). Can add either if you'd rather.
Please do add it to the docstrings! If there are any parameters from __init__ missing in its docstrings than there is something wrong there!
Also add seed to to_dict and from_dict, please.
Otherwise this is a great PR!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1052 +/- ##
===========================================
+ Coverage 81.75% 81.79% +0.03%
===========================================
Files 119 119
Lines 15192 15202 +10
===========================================
+ Hits 12420 12434 +14
+ Misses 2772 2768 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Address review feedback: document the seed argument in every sensor __init__
(this also fills in the name entry that was missing from Gyroscope's docstring),
and carry seed through to_dict/from_dict so it survives serialization, including
GnssReceiver's custom to_dict. from_dict reads it with data.get("seed") so dicts
saved before this change still load, defaulting to None.
Add serialization tests: the seed round-trips through the JSON encoder for every
sensor type, and from_dict defaults the seed to None when the key is absent.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
|
Thanks, done. I added the Also added tests: the seed round-trips through the JSON encoder for every sensor type, plus a case for the missing-key default. I went through the encoder rather than a bare |
Add tests for the argument-validation error paths (measurement range, orientation, vectorized inputs, export file_format) and the __repr__ / __call__ helpers on Sensor and InertialSensor. These paths were untested; with them rocketpy/sensors reaches full statement coverage. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Pull request type
Checklist
ruff checkandruff format --check; pylint 10/10 on the touched files)CHANGELOG.mdupdatedCurrent behavior
Sensor measurement noise is drawn from the process-global NumPy RNG (
np.random.normal), so there's no way to seed it. Noisy-sensor runs aren't reproducible, and the noise is fragile under parallel or forked execution where workers share or reset the global RNG. This is #1042.New behavior
Adds an optional
seedargument to the sensors, threaded throughSensor,InertialSensor,ScalarSensorand the four concrete sensors. Each sensor builds its ownnumpy.random.Generatorfromnp.random.default_rng(seed)and draws its white noise and random walk from that instead ofnp.random.seed=None(the default) keeps the noise random but per-instance, so existing behavior is unchanged unless you pass a seed. With a seed, the noise is reproducible and independent of the global RNG state._resetis left alone on purpose (no re-seed): re-seeding on every reset would make each run in a repeated or Monte Carlo loop draw identical noise, which isn't the goal.tests/unit/sensors/test_sensor_seeding.pycovers same-seed reproducibility, decorrelation across seeds, independence from the global RNG (and not consuming it, which is the regression guard for the original bug), and the GnssReceiver path.Breaking change
With
seed=Nonethe output is identical to before; the argument is opt-in.Additional information
I kept
seedout of the docstrings andto_dict/from_dictto keep the change minimal and consistent with the existing style (a couple of the sensor__init__s don't document every parameter either). Can add either if you'd rather.Closes #1042