a87bdf73e7a8 — Chris Cannam 2 months ago
Important fix to calculateAdvance - don't advance both if row is just one short, as this can happen spuriously
1 files changed, 15 insertions(+), 8 deletions(-)

M constrained-dtw-fn.sml
M constrained-dtw-fn.sml +15 -8
@@ 202,7 202,7 @@ functor ConstrainedDTWFn (ARG : CONSTRAI
         : advance_direction =
         let open DTWLog
             val () = debug (fn () => ["calculateAdvance: nx = %1, ny = %2, target x = %3, y = %4, row length %5, column length %6, search width %7", I nx, I ny, I x, I y, I (CVR.length (#costs row)), I (CVR.length (#costs column)), I (#searchWidth dtw)])
-            val () = checkConsistent face
+            (* val () = checkConsistent face *)
         in
             if x + 1 >= nx
             then if y + 1 >= ny

          
@@ 210,12 210,19 @@ functor ConstrainedDTWFn (ARG : CONSTRAI
                  else ADVANCE_Y
             else if y + 1 >= ny
             then ADVANCE_X
-            else if CVR.length (#costs row) < #searchWidth dtw
-            then ADVANCE_BOTH
-            else case ARG.Cost.compare (minCost row, minCost column) of
-                     LESS => ADVANCE_Y
-                   | GREATER => ADVANCE_X
-                   | _ => ADVANCE_BOTH
+            else
+                (* The +1 is necessary here because when we
+                   ADVANCE_BOTH we do them one after the other rather
+                   than simultaneously, and so one of them is left a
+                   cell shorter. So without this we can end up
+                   returning ADVANCE_BOTH every time because it always
+                   looks as if one is incomplete. *)
+                if CVR.length (#costs row) + 1 < #searchWidth dtw
+                then ADVANCE_BOTH
+                else case ARG.Cost.compare (minCost row, minCost column) of
+                         LESS => ADVANCE_Y
+                       | GREATER => ADVANCE_X
+                       | _ => ADVANCE_BOTH
         end
             
     fun advance (dtw : dtw)

          
@@ 373,7 380,7 @@ functor ConstrainedDTWFn (ARG : CONSTRAI
                           (acc : int list)
                           (loc as { x, y } : location)
                 : int list =
-                let val () = debug (fn () => ["backward': location %1, acc is %2", locationToString loc, SL (map I acc)])
+                let val () = () (* debug (fn () => ["backward': location %1, acc is %2", locationToString loc, SL (map I acc)]) *)
                 in
                     case peelBackTo loc m of
                         [] => (debug (fn () => ["backward': returning %1", SL (map I acc)]);