Skip to content

Fix CompatibleRvtMotion NaN propagation when frequency range exceeds target spectrum bound - #28

Merged
arkottke merged 1 commit into
mainfrom
fix-compatible-rvt-high-freq-nan
Sep 10, 2026
Merged

Fix CompatibleRvtMotion NaN propagation when frequency range exceeds target spectrum bound#28
arkottke merged 1 commit into
mainfrom
fix-compatible-rvt-high-freq-nan

Conversation

@arkottke

@arkottke arkottke commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Problem

A Response-Spectrum-Compatible RVT motion silently fails to compute (no curve drawn, no crash) when:

  • The target response spectrum's shortest period is exactly 0.01 s (100 Hz).
  • The motion's frequency dimension max is set above 100 Hz.

Root cause

In CompatibleRvtMotion::calculate(), the high-frequency extrapolation region (highOffset) anchors to _fourierAcc[highOffset - 1]. When highOffset is 0 this 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 whose log() 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

  • Added a safeFasForLog()/kFasFloor helper to sanitize FAS values before log().
  • Bounds-guarded highOffset/offset indexing 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.
  • Fixed the high-frequency slope-sign check to also treat NaN as invalid (a plain > 0 comparison never catches NaN).
  • Added 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 instead of failing silently.

Notes

  • Investigated switching gsl_interp_csplinegsl_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.
  • Did not add a new example/ regression case: compare_examples.py requires an existing reference outputCatalog generated by actually running the compiled binary (--update mode), which isn't possible without a local Qt6/GSL build. All 5 existing CompatibleRvtMotion examples use freq.max == targetMaxFreq == 100 Hz exactly (the boundary), so none currently exercise freq.max > targetMaxFreq. Follow-up: a maintainer with a local build could add such a case.

…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>
@arkottke
arkottke merged commit 602c35d into main Sep 10, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant