BUG: Fire IterationEvent before AdvanceOneStep in v4 gradient optimizers (closes #2570, supersedes #2572)

[hjmjohnson]

Fixes #2570 and supersedes the WIP draft #2572 by @gregsharp. The 2021 PR stalled on coding-standards review, not on the diagnosis — Greg was right about the bug.

GradientDescentOptimizerv4 (and its subclasses) fired IterationEvent at the end of AdvanceOneStep(), after UpdateTransformParameters had already moved the position from X to X+step, while m_CurrentMetricValue still held v(X). Observers reading GetCurrentMetricValue() and GetCurrentPosition() got an inconsistent pair (old value with new position).

This PR moves the event invocation to the loop in ResumeOptimization(), fired BEFORE AdvanceOneStep(), so observers always see the value at the position where it was actually computed. Six-line change to the base; trailing InvokeEvent removed from each subclass override (RegularStep, QuasiNewton, GradientDescentLineSearch, ConjugateGradientLineSearch).

User-visible symptom (from issue #2570)

The reproducer (a plastimatch v4-registration test program and its AmoebaOptimizerv4 vs RegularStepGradientDescentOptimizerv4 comparison) produces this observer output at iteration 0:

expected:  0   17208.6   [0, 0, 0]
actual:    0   17208.6   [14.999996789911513, -0.006939114154077091, -0.006939116585946947]

The score at iteration 0 is the score evaluated at the initial parameters [0, 0, 0] — but the position the observer reads has already been advanced one step. Same value, different position, no way for the observer to reconstruct what happened. Discussed on Discourse: imageregistrationmethodv4-questions/4117/7.

TDD evidence — failing test pinned the bug, then passes after fix

The new GTest itkGradientDescentOptimizerv4ObserverGTest.cxx attaches an observer at IterationEvent and asserts that metric_at(reported_position) == reported_value. On unfixed upstream/main:

GradientDescentOptimizerv4.ObserverReportsConsistentValueAndPosition (FAILED)
RegularStepGradientDescentOptimizerv4.ObserverReportsConsistentValueAndPosition (FAILED)
  Iteration 0: observer reported value 23999.99... at position [99.998..., -99.992...]
               but metric at that position is 23995.96... (off by ~4)

After the fix:

[ OK ] GradientDescentOptimizerv4.ObserverReportsConsistentValueAndPosition (0 ms)
[ OK ] RegularStepGradientDescentOptimizerv4.ObserverReportsConsistentValueAndPosition (0 ms)

Why this approach over Greg's #2572 restructure

Greg's PR rewrote ResumeOptimization to compute the metric value/derivative after AdvanceOneStep (~150 lines changed across 7 files). It's correct, but it changes the iteration semantics:

Old: zero iterations = no evaluations; one iteration = one evaluation + one position update.
Greg's: zero iterations = no evaluations; one iteration = one evaluation but no position update.

@dzenanz pushed back on that semantic flip in the 2021 review, and the discussion never resolved. This PR is the minimal alternative: keep the existing "1 iteration = 1 step taken" semantic and move the event invocation 6 lines earlier in the loop. No iteration-semantic change, no risk of breaking dependent registration code.

Other items in Greg's branch — explicit zero-init of m_Gradient / m_PreviousGradient in StartOptimization, an extra m_PreviousGradient = m_Gradient after the initial evaluation — were inspected for first-iteration correctness concerns. With the event-firing fix in place, neither is load-bearing: subclasses (RegularStepGradientDescentOptimizerv4) already initialize their own gradient buffers, and the relaxation dot product on iteration 0 is unchanged either way. They're omitted here to keep the surface minimal.

Files changed

• Modules/Numerics/Optimizersv4/include/itkGradientDescentOptimizerv4.hxx — add InvokeEvent(IterationEvent()) before AdvanceOneStep() in ResumeOptimization; remove the trailing InvokeEvent from AdvanceOneStep
• Modules/Numerics/Optimizersv4/include/itkRegularStepGradientDescentOptimizerv4.hxx — remove trailing InvokeEvent from AdvanceOneStep
• Modules/Numerics/Optimizersv4/include/itkQuasiNewtonOptimizerv4.hxx — same
• Modules/Numerics/Optimizersv4/include/itkGradientDescentLineSearchOptimizerv4.hxx — same
• Modules/Numerics/Optimizersv4/include/itkConjugateGradientLineSearchOptimizerv4.hxx — same
• Modules/Numerics/Optimizersv4/test/itkGradientDescentOptimizerv4ObserverGTest.cxx — new (regression guard)
• Modules/Numerics/Optimizersv4/test/CMakeLists.txt — register the new GTest

Verification

• All 21 tests in Optimizersv4 + RegistrationMethodsv4 labels pass locally on the fix
• pre-commit run --all-files exit 0
• New GTest fails on upstream/main (no fix), passes on this branch — confirms it actually guards against the bug

[hjmjohnson]

@ntustison @stnava @blowekamp @cookpa

This issue has been outstanding for a long time. The ANTs may have behavior that depends on this, so I'm giving you a chance to review before it is merged in the next few days.

[cookpa]

Thanks for the heads up @hjmjohnson. I did a bit of checking with Codex and I think there will be no problems with ANTs. It threw out the suggestion I put in the comment above just as a general point