Fix CompatibleRvtMotion NaN propagation when frequency range exceeds target spectrum bound - #28
Merged
Merged
Conversation
…rget spectrum When the target response spectrum's shortest period is exactly the interpolate/extrapolate boundary (e.g. 0.01 s = 100 Hz) and the motion's frequency dimension max is set above that, the high-frequency extrapolation could read out-of-bounds Fourier amplitude values (undefined behavior when highOffset == 0/1) or take log() of a non-positive/non-finite value from a cubic-spline boundary overshoot. The resulting NaN silently propagated through the rest of the extrapolated tail, producing a blank plot with no error. - Add safeFasForLog()/kFasFloor helper to sanitize values before log(). - Bounds-guard highOffset/offset indexing in both the initial FAS estimate and the ratio-correction extrapolation loops, with a documented fallback anchor when the frequency range fully exceeds the target spectrum's edge. - Fix the low/high-frequency slope checks to also treat NaN as invalid (a plain `> 0` comparison never catches NaN). - Add CompatibleRvtMotion::sanitizeFourierAcc(), called after each FAS update, to replace any remaining non-finite/non-positive values with a small positive floor and emit a qWarning() diagnostic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Problem
A Response-Spectrum-Compatible RVT motion silently fails to compute (no curve drawn, no crash) when:
0.01 s(100 Hz).Root cause
In
CompatibleRvtMotion::calculate(), the high-frequency extrapolation region (highOffset) anchors to_fourierAcc[highOffset - 1]. WhenhighOffsetis0this reads index-1(undefined behavior in Release builds). Separately, a natural cubic-spline can overshoot right at the target spectrum's period-domain boundary, producing a non-positive FAS estimate whoselog()is NaN — that NaN then poisons the entire extrapolated tail, and Qwt silently omits NaN samples from the plot, matching the reported "no curve was drawn" symptom without any crash or error.Fix
safeFasForLog()/kFasFloorhelper to sanitize FAS values beforelog().highOffset/offsetindexing in both the initial FAS estimate loop and the ratio-correction extrapolation loop, with a documented fallback anchor for the degenerate case where the whole frequency range lies above the target spectrum's highest analyzed frequency.> 0comparison never catches NaN).CompatibleRvtMotion::sanitizeFourierAcc(), called after each FAS update, to replace any remaining non-finite/non-positive values with a small positive floor and emit aqWarning()diagnostic instead of failing silently.Notes
gsl_interp_cspline→gsl_interp_akima; decided against it since it would change numerical output for every existing example and can't be safely validated without a full local build.example/regression case:compare_examples.pyrequires an existing referenceoutputCataloggenerated by actually running the compiled binary (--updatemode), which isn't possible without a local Qt6/GSL build. All 5 existing CompatibleRvtMotion examples usefreq.max == targetMaxFreq == 100 Hzexactly (the boundary), so none currently exercisefreq.max > targetMaxFreq. Follow-up: a maintainer with a local build could add such a case.