Skip to content

Fix/alp time loop termination#224

Merged
tobre1 merged 2 commits into
ViennaTools:masterfrom
philipphaslhofer:fix/alp-time-loop-termination
Apr 10, 2026
Merged

Fix/alp time loop termination#224
tobre1 merged 2 commits into
ViennaTools:masterfrom
philipphaslhofer:fix/alp-time-loop-termination

Conversation

@philipphaslhofer

Copy link
Copy Markdown
Contributor

Summary

The ALP pulse-time loop had two bugs:

  1. Broken termination condition. while (std::fabs(time - pulseTime) > 1e-6) only exits
    when time lands within 1e-6 of pulseTime. For any pulseTime / coverageTimeStep that
    is not an exact integer, time overshoots and the fabs keeps growing — the loop never
    exits. Only the "neatly divisible" case worked.
  2. Last-step overshoot. The final iteration runs fluxes and
    coverage updates with the full coverageTimeStep, so the physics integrates over more time
    than actually remains in the pulse.

Changes

  • Commit 1 — replace the fabs guard with a monotonic time < pulseTime - coverageTimeStep * 1e-4 check so the loop terminates for arbitrary pulseTime / coverageTimeStep ratios and is invariant of the time scale.
  • Commit 2 — clamp dt = min(coverageTimeStep, pulseTime - time) and push it to the
    surface model via a new setTimeStep hook on SurfaceModel, so the final iteration
    integrates over the actual remaining time. SingleParticleALDSurfaceModel overrides the
    hook to update its (now non-const) dt_; base-class default is a no-op so other models are
    unaffected.

  The pulse-time loop in ALPStrategy used an absolute difference
  and hardcoded absolute epsilon (std::fabs(time - pulseTime) > 1e-6)
  as its termination guard, which does not brake if the time increment is not a whole fraction of pulseTime (or very close).

  Replace with a scaled epsilon (coverageTimeStep * 1e-4) and changed termination guard.
  Clamp dt = min(coverageTimeStep, pulseTime - time) and push it to the
  surface model via a new setTimeStep hook to non-const dt_ so the final iteration
  integrates over the actual remaining pulse time instead of overshooting
  with the full coverageTimeStep.
@philipphaslhofer
philipphaslhofer force-pushed the fix/alp-time-loop-termination branch from acc1f4f to ed84ffc Compare April 10, 2026 10:58
@philipphaslhofer philipphaslhofer added the enhancement New feature or request label Apr 10, 2026
@tobre1
tobre1 merged commit a83140f into ViennaTools:master Apr 10, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants