Fix InflowWind Grid3D cubic velocity interpolation NaN bug - #3428
Fix InflowWind Grid3D cubic velocity interpolation NaN bug#3428andrew-platt wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes InflowWind Grid3D cubic-in-time velocity interpolation behavior so callers that request velocity-only (no acceleration output) no longer receive NaN/Inf wind velocities, and corrects the Grid3D acceleration precompute guard to use the number of time steps rather than the tower-grid count.
Changes:
- Ensure
Grid3DField_GetCellpopulatesAccCellwheneverVelInterpCubicis enabled, independent of whetherAccelUVWis allocated by the caller. - Fix
IfW_Grid3DField_CalcAccelearly-return condition to checkG3D%NSteps < 3(time steps) instead ofG3D%NTGrids < 3(tower grid points). - Add and register new InflowWind unit-test coverage for the cubic-velocity-only bug path.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| unit_tests/CMakeLists.txt | Adds the new Grid3D unit test source to the InflowWind unit test target. |
| modules/inflowwind/tests/test_grid3d_field.F90 | Introduces a new unit test module exercising the cubic velocity interpolation path. |
| modules/inflowwind/tests/inflowwind_utest.F90 | Registers the new Grid3D test suite in the InflowWind unit test runner. |
| modules/inflowwind/src/IfW_FlowField.f90 | Fixes the Grid3D cubic interpolation path selection and corrects the CalcAccel guard condition. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
IfW_FlowField_GetVelAcc only populated the local AccCell array when the caller requested acceleration output (OutputAccel). But the cubic Hermite velocity formula in Grid3DField_GetVelAccCubic depends on AccCell for its tangent terms regardless of whether acceleration is separately requested. Callers that use VelInterpCubic=True but only want velocity (e.g. AWAE's ambient-wind sampling) therefore computed velocity from uninitialized memory, corrupting every returned value. Also fixes IfW_Grid3DField_CalcAccel, which checked G3D%NTGrids (tower grid point count) instead of G3D%NSteps (time step count) to decide whether to compute real cubic-spline time derivatives, forcing G3D%Acc to zero for every case without a tower file regardless of how many time steps were available. Adds unit tests (test_grid3d_field.F90) that reproduce both defects in isolation and verify the fix. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
f6aaeb6 to
2942295
Compare
|
| Channel | Max |diff| | Mean |diff| | Old max magnitude | New max magnitude |
|---|---|---|---|---|
T, X, Y, Z, U, V, W |
0.0 | 0.0 | — | — |
UA |
10.16 | 1.30 | 5.37 | 13.57 |
VA |
11.97 | 1.39 | 5.57 | 15.50 |
WA |
9.26 | 1.28 | 7.97 | 12.63 |
The new accelerations are ~2-3x larger in typical magnitude, consistent with the old code systematically biasing derivatives toward zero rather than a small numeric drift.
…3d-cubic-interp-nan
50613f4 to
cada939
Compare
IfW_FlowField_GetVelAcc only populated the local AccCell array when the caller requested acceleration output (OutputAccel). But the cubic Hermite velocity formula in Grid3DField_GetVelAccCubic depends on AccCell for its tangent terms regardless of whether acceleration is separately requested. Callers that use VelInterpCubic=True but only want velocity (e.g. AWAE's ambient-wind sampling) therefore computed velocity from uninitialized memory, corrupting every returned value. Also fixes IfW_Grid3DField_CalcAccel, which checked G3D%NTGrids (tower grid point count) instead of G3D%NSteps (time step count) to decide whether to compute real cubic-spline time derivatives, forcing G3D%Acc to zero for every case without a tower file regardless of how many time steps were available. Adds unit tests (test_grid3d_field.F90) that reproduce both defects in isolation and verify the fix. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…an' into bugfix/ifw-grid3d-cubic-interp-nan
Ready to merge
Feature or improvement description
This is a bug fix targeting
rc-5.0.1, not a feature. It fixes a long-standing defect in InflowWind'sGrid3Dflow field (TurbSim/Bladed/HAWC full-field wind files,WindType=3and similar) that produces NaN wind velocities wheneverVelInterpCubic = Trueis set in the InflowWind input file and the calling code queries velocity without also requesting acceleration output. FAST.Farm's AWAE module is exactly such a caller: it samples ambient wind velocity for its high-resolution grid without asking for acceleration, so any FAST.Farm case withVelInterpCubic = Truereturns a 100%-NaN ambient wind field and fails immediately at t=0 with:Root cause
IfW_FlowField_GetVelAcc(inmodules/inflowwind/src/IfW_FlowField.f90) determines whether the caller wants acceleration output from whether it allocated the optionalAccelUVWoutput array:OutputAccel = allocated(AccelUVW)For the
Grid3D_FieldTypecase, this flag (combined withVelInterpCubic) selects one of four interpolation code paths. WhenVelInterpCubic = TrueandOutputAccel = False("cubic velocity, no acceleration"),Grid3DField_GetCellis called withCalcAccel = OutputAccel = .false., which causes it to skip populating the localAccCell(8,3)array entirely — it is only filledif (CalcAccel) then ....The velocity is then computed by
Grid3DField_GetVelAccCubic, which implements a cubic Hermite spline in time. Critically, the Hermite value formula (not just the optional acceleration output) depends onAccCell, since a proper cubic-in-time interpolant needs derivative (tangent) information at both time endpoints to compute the interpolated value itself:where
PPis derived fromAccCell. SinceAccCellis a local, stack-allocated array that is never initialized in this code path,PPis computed from garbage memory, corruptingVelocityfor every single query — confirmed via debugger to containInfin every element for this test case, while the companionVelCell(actual wind data) was fully valid. This explains why the reported failure showed a 100%-NaN velocity array with a 100%-valid position array: the defect is purely a function of which interpolation-flag combination is selected, not of any particular spatial location.A second, related defect was found in the same area.
IfW_Grid3DField_CalcAccel(which precomputes the time-derivative arrayG3D%Accneeded for cubic interpolation) has:G3D%NTGridsis the number of tower grid points (from an optional tower file), not the number of time steps (G3D%NSteps). For the very common case of no tower file (NTGrids = 0), this unconditionally forcesG3D%Accto zero and skips the real cubic-spline derivative calculation — regardless of how many time steps are actually available (4004 in the reported case). This doesn't itself produce NaN (zero is well-defined), but it silently defeats cubic-in-time interpolation whenever no tower file is present, which is likely most cases that useVelInterpCubic = True.Both defects are pre-existing in
rc-5.0.1(and earlier) — reproduced and confirmed on an unmodifiedrc-5.0.1checkout with identical input files, so this is not a regression from any other branch. It is essentially untested in the existing test suite because every other reg-test input file in the repository setsVelInterpCubic = False; the reported case is the only one usingTrue.Fix
IfW_FlowField_GetVelAcc: passOutputAccel .or. FF%VelInterpCubicas theCalcAccelargument toGrid3DField_GetCell, soAccCellis always populated whenever cubic-in-time interpolation is active, independent of whether the caller also wants acceleration returned.IfW_Grid3DField_CalcAccel: change the early-return guard fromG3D%NTGrids < 3toG3D%NSteps < 3, so real time derivatives are computed whenever enough time samples exist, regardless of whether a tower file is present.Ready to merge
Related issue, if one exists
None filed yet; found during investigation of a reported FAST.Farm
FARM_InitialCOfatal error ("rotor plane has left the low-resolution domain") on a 2-turbine wind-tunnel test case.Impacted areas of the software
modules/inflowwind/src/IfW_FlowField.f90(IfW_FlowField_GetVelAcc,IfW_Grid3DField_CalcAccel)Grid3Dflow field type (TurbSim, Bladed, HAWC full-field wind,WindType = 3, 4, 5, 7) withVelInterpCubic = True: FAST.Farm (AWAE ambient-wind sampling and per-turbine InflowWind instances), OpenFAST, InflowWind driver, AeroDyn driver.VelInterpCubic = Falselinear-interpolation path.Additional supporting information
Diagnosed via GDB by tracing a NaN observed in AeroDyn's
RotInflow%Blade%InflowVelbackwards throughFASTWrapper, AWAE's ambient-wind fill, and into InflowWind'sGrid3Dcubic interpolation, whereAccCellwas directly observed to containInfin all elements at the point of use. Confirmed the same failure reproduces on an unmodifiedrc-5.0.1build with AMReX disabled, ruling out any connection to unrelated work-in-progress changes on another branch (including a separately-identified, unrelatedWakeDynamicsarray-bounds issue, which is not part of this PR).Generative AI usage
Root-cause investigation (GDB tracing, source review), the code fix, and the new unit tests were developed with substantial assistance from an AI coding agent.
Co-authored-by: Claude Sonnet 5 noreply@anthropic.com
Test results, if applicable
modules/inflowwind/tests/test_grid3d_field.F90with two new unit tests, registered ininflowwind_utest:test_grid3d_cubic_vel_only— builds a minimal syntheticGrid3Dfield with a spatially-uniform, linear-in-time velocity ramp andVelInterpCubic = True; queries velocity both with and without requesting acceleration output. Fails today with "not finite (NaN/Inf)" without fix AeroDyn14 Driver #1; passes with it.test_grid3d_calcaccel_no_tower— callsIfW_Grid3DField_CalcAcceldirectly on a field with no tower grid and checks that the computed derivative recovers the known true value. Fails today (derivative forced to zero) without fix DWM Driver #2; passes with it.inflowwind_utestsuite passes with no regressions in any pre-existing test.