Skip to content

Procrustes outer-loop "got worse" check compares objectives across different templates #265

@fabian-s

Description

@fabian-s

Follow-up. The Procrustes outer loop in R/register.R:540-599 has an early-stop ("alignment worsened") at lines ~570-579 that compares obj of the current iteration's alignment (computed against current_template) with best_obj from a previous iteration's alignment (computed against the old template). Objectives across different templates aren't directly comparable.

Consequences

  • The loop can bail spuriously when the alignment is in fact improving against the refined template
  • Or it can stay one iteration past the actual best
  • For method = "srvf" with a user template, the outer objective falls through to plain L2 (tf_integrate((aligned - template)^2)) while the inner alignment minimizes the elastic/Fisher–Rao criterion — "worsening" in L2 is entirely compatible with improvement in the actual criterion being optimized
  • Per-curve tf_integrate returns NA for curves with boundary NAs (affine), so mean(..., na.rm = TRUE) computes the objective over a changing subset of curves between iterations

Suggested fix

Evaluate both candidate objectives against the same (current) template before comparing. Use an amplitude-distance objective for SRVF (not L2). Handle NA curves consistently across iterations (drop them once and keep them dropped).

Found during the June-2026 ground-up review; out-of-scope for #242's fix but worth fixing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions