fix(ode): validate ExponentialMap::integrate_attitude's timestep and … - #307
Open
naseem173 wants to merge 1 commit into
Open
fix(ode): validate ExponentialMap::integrate_attitude's timestep and …#307naseem173 wants to merge 1 commit into
naseem173 wants to merge 1 commit into
Conversation
…rate Fixes kmolan#302. attitude_step and attitude_step_with_angular_acceleration fed a timestep and angular rate straight into SO3::exp with no validation: a non-finite rate produced a quaternion of NaNs that never recovered, and a non-positive timestep integrated backwards (or not at all) without comment. attitude_step and attitude_step_with_angular_acceleration stay infallible, since they sit on RigidBody::stepped's documented panic-free per-tick path (and estimation::attitude_correction's per-tick path) - making them fallible would force that hot loop to become fallible too. Their docs now carry an explicit "Behavior" note describing the NaN policy instead of leaving it silent. integrate_attitude, the multi-step driver (not used on any per-tick hot path, and already the same shape as the fallible Rk45::solve), now validates dt up front and the rate returned by its angular_rate_at callback on every step, returning the new IntegrateError::NonPositiveTimestep or the existing IntegrateError::NonFinite instead of silently producing garbage.
Owner
|
@naseem173 thanks for the contribution, please address existing failures before I review the changes. Looks like you need to run |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…rate
Fixes #302. attitude_step and attitude_step_with_angular_acceleration fed a timestep and angular rate straight into SO3::exp with no validation: a non-finite rate produced a quaternion of NaNs that never recovered, and a non-positive timestep integrated backwards (or not at all) without comment.
attitude_step and attitude_step_with_angular_acceleration stay infallible, since they sit on RigidBody::stepped's documented panic-free per-tick path (and estimation::attitude_correction's per-tick path) - making them fallible would force that hot loop to become fallible too. Their docs now carry an explicit "Behavior" note describing the NaN policy instead of leaving it silent.
integrate_attitude, the multi-step driver (not used on any per-tick hot path, and already the same shape as the fallible Rk45::solve), now validates dt up front and the rate returned by its angular_rate_at callback on every step, returning the new IntegrateError::NonPositiveTimestep or the existing IntegrateError::NonFinite instead of silently producing garbage.
What & why
Adds input validation to
ExponentialMap::integrate_attitudeper #302, without changing the infallible signature of the per-tick primitives it's built from. See the commit message above for the full reasoning on scope.Checklist
cargo test+cargo clippy --all-targetsclean locally (not run locally in this environment — relying on CI to verify; happy to fix any failures)unwrap/expect/panicon library paths (typed errors instead)