Skip to content

BeamDyn: return DTBeam from BD_Init and sub-step the driver; error checks on registry checkpoint reads/writes - #3517

Open
andrew-platt wants to merge 10 commits into
OpenFAST:rc-5.0.1from
andrew-platt:b/BD_driver_dt
Open

andrew-platt wants to merge 10 commits into
OpenFAST:rc-5.0.1from
andrew-platt:b/BD_driver_dt

Conversation

@andrew-platt

@andrew-platt andrew-platt commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

Ready to merge after CI testing

Feature or improvement description

This PR fixes the three problems reported in #3516. It also fixes a module-indexing error in the glue code's tight-coupling sub-step check, found while working on item 1.

1. A numeric DTBeam was not seen by the BeamDyn driver

BD_Init used its Interval argument only as the default for DTBeam and never returned the time step it used. The driver kept stepping inputs and output times with its own dt while the integrator advanced by DTBeam. With dt = 0.002 and DTBeam = 0.001, bd_5MW_dynamic gave N1Fxl = -3.8e5 at t = 0.004 s instead of about 7.0e3, with no message.

  • BD_Init now returns p%dt through Interval, as the Interval documentation says it should.
  • beamdyn_driver now sub-steps BeamDyn. If DTBeam is an integer divisor of the driver dt, it takes dt/DTBeam BeamDyn steps per driver step and still writes output and VTK every dt. It stops with an error if DTBeam is larger than dt or does not divide it. Static analysis is not sub-stepped (from Copilot review): it does not use DTBeam, since each driver step is a load step.
  • OpenFAST, loose coupling (ModCoupling = 1): the glue code already sub-steps modules at the time step returned from their Init (MV_AddModule / FAST_UpdateStates). Before, it always got DT back from BeamDyn, used a single sub-step, and BeamDyn integrated by DTBeam: the same silent mismatch as in the driver. Now BeamDyn is sub-stepped correctly. A DTBeam larger than DT, or one that does not divide it, is rejected by MV_AddModule.
  • BD_UpdateStates now gives BD_UpdateGlobalRef the inputs interpolated to t + p%dt when BeamDyn is being sub-stepped (from Copilot review). Before, it passed u(1), which during glue-code sub-stepping is the root motion at the end of the full glue-code step, so intermediate sub-steps updated the reference frame and root node from future root motion. When u(1) is already at t + p%dt (no sub-stepping, and the driver), u(1) is used as before. In the test below this made no measurable difference to the results.
  • OpenFAST, tight coupling (ModCoupling = 2, 3): the glue code integrates the BeamDyn states at DT and does not call BD_UpdateStates, so DTBeam is not used. The solver now prints its existing "Sub-stepping disabled" info message for BeamDyn (see item 4).
  • A zero or negative DTBeam is now rejected (from Copilot review). BD_ValidateInputData requires DTBeam > 0 for dynamic solves. Static solves skip the check, since they don't use DTBeam and the driver dt may be 0 there. The driver also rejects a nonpositive BeamDyn step before computing sub-steps. Before, zero divided by zero, and a negative value gave a nonpositive sub-step count, so the beam states never updated while output time advanced.
  • The DTBeam entry in the BeamDyn input-file docs now states these rules.

2. Registry unpack reads had no error return

None of the generated read(RF%Unit) statements in ModReg.f90 had iostat. A truncated checkpoint stopped the program with a Fortran runtime error instead of setting RF%ErrStat, which kills the calling process when OpenFAST is used as a library (C/Python). #3486 checks the file size before unpacking, but that does not cover other read errors. Despite what the issue says, the write(RF%Unit) statements had no iostat either.

ModRegGen.py now generates iostat checks on the data-number and data reads and writes, setting RF%ErrStat/RF%ErrMsg on failure. ModReg.f90 is regenerated from it. Before the change, the generator reproduced the committed file exactly, so the only differences in ModReg.f90 come from the generator change.

RegUnpackPointer also used the unpacked index to look up RF%Pointers without checking for a read error (from Copilot review). With the new iostat checks, or an existing data-number mismatch, the index is undefined at that point. A corrupt index was also never range-checked, so a bad checkpoint could still crash the calling process. RegUnpackPointer now initializes its outputs, returns if the read failed, and sets an error if the index is outside RF%Pointers.

3. BD_End closed the summary file without resetting the unit

BD_End now sets m%Un_Sum = -1 after closing the file. A caller that reuses the same misc variables can then no longer close a stale unit a second time.

4. Tight-coupling sub-step check used the wrong module index (FAST_Solver.f90)

The loop that prints "Sub-stepping disabled for this module" for tightly coupled modules indexed GlueModData(i) instead of GlueModData(p%iModTC(i)). So it checked and named the first size(p%iModTC) modules in GlueModData rather than the tightly coupled ones. All the other category loops above it already use p%iModTC(i). Only the info message is affected.

Related issue, if one exists

Closes #3516
Related: #3486 (BeamDyn C bindings). Item 2 only shows up through the C/Python interface added there.

Impacted areas of the software

  • BeamDyn: BD_Init, BD_End, BD_ValidateInputData (DTBeam > 0), BD_UpdateStates (reference frame update when sub-stepped)
  • BeamDyn driver: time stepping when DTBeam is numeric
  • OpenFAST glue code: BeamDyn sub-stepping with loose coupling (through the returned DTBeam); tight-coupling sub-step info message
  • NWTC Library: ModRegGen.py / ModReg.f90 (checkpoint pack/unpack used by every module's registry routines)
  • Docs: BeamDyn DTBeam description
  • Regression tests: new bd_5MW_dynamic_substep case (reg_tests/CTestList.cmake, r-test)

Additional supporting information

  • The first four commits are one per item, in the order above. A fifth commit removes an OpenFAST DTBeam == DT check that the first commit had added. It turned out not to be needed for either coupling mode. The last four commits address the Copilot review: rejecting a nonpositive DTBeam (item 1), the error and range checks in RegUnpackPointer (item 2), no sub-stepping for static analysis in the driver (item 1), and the interpolated inputs for BD_UpdateGlobalRef (item 1).
  • All 20 BeamDyn primary input files in r-test use DTBeam = "DEFAULT". In that case Interval comes back unchanged, and the driver and glue code take a single BeamDyn step per step. Regression results should not change.
  • OpenFAST cases with a numeric DTBeam different from DT now sub-step BeamDyn (loose coupling) or print an info message that DTBeam is not used (tight coupling). Before, they ran with an inconsistent time step and no message.
  • Known limitation, not fixed here: in OpenFAST with InterpOrder = 2, a sub-stepped BeamDyn gets a wrong root motion during the first glue-code step, which gives a large start-up transient in the root loads (see test results). The glue code fills the input history before the start time with copies of the initial input, so quadratic interpolation to the intermediate sub-step times is biased. InterpOrder = 1 is not affected, and neither is the BeamDyn driver. This is a glue-code problem that is not specific to BeamDyn; see Sub-stepped modules get a wrong input on the first glue-code step when InterpOrder = 2 #3521.
  • No new tests are added.

Generative AI usage

Claude Code (claude-opus-5-5) was used to read the issue, make the changes, run the checks below, and draft this description. The changes were reviewed by the author.

Assisted-by: Claude Code:claude-opus-5-5 [Bash] [Read] [Edit]

Development testing

Built beamdyn_driver and openfast (gfortran, Release, double precision). Ran bd_5MW_dynamic from r-test, shortened to t_final = 0.02 s:

Driver dt DTBeam N1Fxl at 0.004 s N2Fxl at 0.004 s N1Fxl at 0.02 s
0.002 DEFAULT 6.676e+03 2.593e+03 1.618e+04
0.001 DEFAULT 6.995e+03 2.743e+03 1.524e+04
0.002 0.001 (2 sub-steps) 6.995e+03 2.743e+03 1.524e+04
  • The sub-stepped run matches the dt = 0.001 run exactly at every output time compared. The baseline row matches the values given in BeamDyn: numeric DTBeam not seen by the driver; registry unpack without error return; BD_End summary unit #3516.
  • With DTBeam = 0.0015 and dt = 0.002, the driver stops with "must be an integer divisor of the driver time step".
  • With DTBeam = 0 or DTBeam = -0.001, the driver stops with "DTBeam must be greater than 0." The sub-stepped case gives the same results as before this change.
  • bd_static_cantilever_beam with DTBeam = 0.001 (driver dt = 0.002) gives output identical to DTBeam = DEFAULT, which matches the r-test baseline.

OpenFAST loose-coupling sub-stepping: Damped_Beam_Rotating from r-test with ModCoupling = 1, RotSpeed = 60 rpm, Linearize = False, TMax = 0.3 s. Reference: DT = 0.00075, DTBeam = DEFAULT. Sub-stepped: DT = 0.0015, DTBeam = 0.00075.

Time (s) Channel Reference Sub-stepped, InterpOrder = 1 Sub-stepped, InterpOrder = 2
0.0015 B1RootFyr (N) 1.106e+06 1.106e+06 6.542e+10
0.0015 B1RootMxr (N·m) -3.001e+06 -3.001e+06 -1.891e+11
0.3 B1RootFyr (N) 1.149e+06 1.149e+06 9.521e+05
0.3 B1TipTDyr (m) 2.181e-03 2.181e-03 3.107e-03
  • With InterpOrder = 1, the sub-stepped run agrees with the reference to about 5e-7 (relative) on the root load and tip deflection channels over the whole run. It does so with and without the BD_UpdateGlobalRef change.
  • With InterpOrder = 2, the sub-stepped run has the start-up transient described under "Additional supporting information".
  • Not exercised: the tight-coupling info message for a numeric DTBeam, and the new checkpoint iostat and RegUnpackPointer error paths.
  • The full regression suite was not run.

Regression testing

  • r-test branch merging required (b/BD_driver_dt, adds modules/beamdyn/bd_5MW_dynamic_substep)
    • check when r-test is updated

andrew-platt and others added 4 commits November 10, 2026 14:17
BD_Init used its Interval argument only as the default for DTBeam and
never returned the time step it used. With a numeric DTBeam the
BeamDyn driver kept stepping inputs and outputs at its own dt while the
integrator advanced by DTBeam, giving wrong results with no message
(issue OpenFAST#3516, item 1).

- BD_Init now returns p%dt through Interval.
- beamdyn_driver sub-steps BeamDyn when DTBeam is an integer divisor of
  the driver dt and writes output every driver dt. It stops with an
  error if DTBeam is larger than dt or does not divide it.
- OpenFAST: BeamDyn is tightly coupled and cannot be sub-stepped, so
  FAST_InitializeAll now stops with an error if DTBeam differs from DT.
- Docs: describe the DTBeam requirements for OpenFAST and the driver.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The generated read(RF%Unit) and write(RF%Unit) statements in ModReg.f90
had no iostat, so a truncated or unreadable checkpoint stopped the
program with a Fortran runtime error instead of setting RF%ErrStat.
That ends the calling process when OpenFAST is used as a library
(issue OpenFAST#3516, item 2).

ModRegGen.py now generates iostat checks on the data-number and data
reads/writes, setting RF%ErrStat and RF%ErrMsg on failure. ModReg.f90 is
regenerated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
BD_End closed m%Un_Sum but left the unit number unchanged. If BD_End
ran before the first dynamic step (which normally resets it) and the
same misc variables were reused, the stale unit could be closed again,
possibly belonging to another file (issue OpenFAST#3516, item 3).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The loop that reports sub-stepping being disabled for tightly coupled
modules indexed GlueModData with the loop counter instead of
p%iModTC(i), so it checked and named the wrong modules (the first
size(p%iModTC) entries of GlueModData rather than the tight-coupling
ones). All other category loops already use p%iModTC(i).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unchecked pointer-index read failures can still crash the host, while nonpositive BeamDyn time steps can cause division by zero or silently skip state updates.

2 open issues
What changed in this PR

This PR addresses #3516 by aligning BeamDyn time stepping with its callers, improving registry checkpoint error handling, and preventing stale summary-file unit reuse.

Changes:

  • Return BeamDyn’s actual time step, add standalone-driver sub-stepping, and reject mismatched time steps in OpenFAST.
  • Add checkpoint read/write error reporting in the registry generator and generated routines.
  • Reset the closed summary-file unit and correct module indexing in tight-coupling diagnostics.
File Description
modules/​openfast-library/​src/​FAST_Subs.f90 Reject BeamDyn time steps that differ from OpenFAST’s step.
modules/​openfast-library/​src/​FAST_Solver.f90 Correct module indexing in sub-stepping diagnostics.
modules/​nwtc-library/​src/​ModReg.f90 Add generated checkpoint read/write error checks.
modules/​nwtc-library/​ModRegGen.py Generate registry I/O status checks and error messages.
modules/​beamdyn/​src/​Driver_Beam.f90 Sub-step BeamDyn while preserving the driver’s output cadence.
modules/​beamdyn/​src/​BeamDyn.f90 Return the actual time step and reset the closed summary unit.
docs/​source/​user/​beamdyn/​input_files.rst Document DTBeam requirements for OpenFAST and standalone use.

Review effort: Balanced

Comment thread modules/beamdyn/src/Driver_Beam.f90 Resolved
Comment thread modules/nwtc-library/ModRegGen.py Resolved
andrew-platt and others added 3 commits November 10, 2026 14:30
The check added in b879d65 stopped OpenFAST whenever DTBeam differed
from DT. That is not needed now that BD_Init returns DTBeam:

- Loose coupling: FAST_UpdateStates already sub-steps BeamDyn at its
  own time step, and MV_AddModule rejects a DTBeam that is larger than
  DT or not an integer divisor of it.
- Tight coupling: the glue code integrates the BeamDyn states at DT and
  BD_UpdateStates is not called, so DTBeam is not used. The solver
  already prints an info message that sub-stepping is disabled.

Update the DTBeam documentation to match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A zero or negative numeric DTBeam was not rejected. In the driver,
zero divided by zero when computing sub-steps, and a negative value
gave a nonpositive sub-step count, so the beam states were never
updated while output time advanced.

- BD_ValidateInputData: DTBeam must be > 0 for dynamic solves (DTBeam
  is not used in static solves, where the driver dt may be 0).
- Driver SetBDSubSteps: also reject dt_BD <= 0 before computing the
  number of sub-steps.

Addresses Copilot review on OpenFAST#3517.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RegUnpackPointer used Idx to index RF%Pointers without checking whether
reading it failed. With the new iostat checks (or an existing data
number mismatch), Idx is undefined at that point, and a corrupt index
was never range checked, so a bad checkpoint could still crash the
calling process with an invalid memory access.

RegUnpackPointer now initializes Ptr and Idx, returns if the read set
an error, and sets an error if Idx is outside RF%Pointers. ModReg.f90
is regenerated.

Addresses Copilot review on OpenFAST#3517.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Sub-stepping uses future root motion during intermediate updates, introducing artificial deformation, and makes static analyses perform redundant nonlinear solves.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Static analyses incorrectly use dynamic timestep ratio

modules/​beamdyn/​src/​Driver_Beam.f90:275

Static analyses also reach this branch, even though BD_UpdateStates dispatches to BD_Static, which solves equilibrium rather than advancing by p%dt. For a stationary static case with driver dt = 0.1 and numeric DTBeam = 0.0001, the new loop performs 1,000 equivalent nonlinear solves per output step instead of one. Keep n_substeps = 1 and use dt_global for the driver's time markers in static analyses; only dynamic analyses need the timestep ratio.

2 resolved since last review

Review effort: Balanced

Comment thread modules/beamdyn/src/BeamDyn.f90 Resolved
andrew-platt and others added 2 commits November 10, 2026 14:59
Static analysis does not use DTBeam: each driver step is a load step.
Sub-stepping only repeated the static solve at intermediate load
levels, and the DTBeam checks could reject inputs the static solve
never uses. SetBDSubSteps now uses one step per driver step for
static analysis, without checking DTBeam.

Addresses Copilot review on OpenFAST#3517.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…te when sub-stepping

BD_UpdateStates passed u(1) to BD_UpdateGlobalRef. When the caller
sub-steps BeamDyn (OpenFAST loose coupling with DTBeam < DT), u(1) is
at the end of the caller's step, not at t+dt, so intermediate sub-steps
moved the reference frame and set the root node motion from future root
motion. BD_GA2 already interpolates the inputs to t+dt.

When utimes(1) differs from t+dt, the inputs are now interpolated to
t+dt for BD_UpdateGlobalRef. When they are equal, u(1) is used as
before, so results without sub-stepping do not change.

Addresses Copilot review on OpenFAST#3517.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Existing regressions exercise only one sub-step, leaving multi-sub-step equivalence, preserved driver output cadence, and the static load-step exception without automated coverage.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread modules/beamdyn/src/Driver_Beam.f90 Resolved
Adds a driver regression case with DTBeam = 0.001 and driver
dt = 0.002 so the sub-step loop in beamdyn_driver is covered. The
existing BeamDyn driver cases all use DTBeam = DEFAULT and only take
one BeamDyn step per driver step.

Updates the r-test pointer for the new case.

Addresses Copilot review on OpenFAST#3517.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants