usnistgov / SCTK

Other
208 stars 52 forks source link

Bug in src/sclite/ctm2ctm.c #27

Closed FredSRichardson closed 2 years ago

FredSRichardson commented 3 years ago

I came across this bug while while CTM reference format for some diagnostic runs:

diff --git a/src/sclite/ctm2ctm.c b/src/sclite/ctm2ctm.c
index fa8a0a1..1beea26 100644
--- a/src/sclite/ctm2ctm.c
+++ b/src/sclite/ctm2ctm.c
@@ -486,7 +486,7 @@ int chop_WTOKE_2(WTOKE_STR1 *ref, WTOKE_STR1 *hyp, int Rstart, int Hstart, int R
                           overlap(ref->word[Rend].t1,
                                   ref->word[Rend].t1+ref->word[Rend].dur,
                                   hyp->word[Hend].t1,
-                                 hyp->word[Hend].t1+ref->word[Hend].dur)<0.0){
+                                 hyp->word[Hend].t1+ref->word[Rend].dur)<0.0){
                        for (Rend--;
                             Rend >= Rstart && ref->word[Rend].alternate; ) {
                            Rend--;
arlofaria commented 2 years ago

I'm not familiar with this bug, but looking at how this overlap function is used in other contexts it would seem that the parameter passed on line 489 is indeed different from how it's generally called. The description of the overlap function suggests that this is supposed to be s2_t2, i.e. the end time of the second segment.

~So I think perhaps the bugfix might instead be:~

-                                 hyp->word[Hend].t1+ref->word[Hend].dur)<0.0){
+                                 hyp->word[Hend].t1+hyp->word[Hend].dur)<0.0){

~Does that make sense?~

UPDATE: my proposed change is incorrect, since it'd cause make check to fail the tests. Meanwhile, those tests seem to pass with both the code on master and the change suggested by @FredSRichardson in this issue.

jfiscus commented 2 years ago

Thanks @FredSRichardson for the bug report. @arlofaria has the correct fix and the repair is in https://github.com/usnistgov/SCTK/commit/82955db792aefb86a768e5c6c74ecb3f04198a95. This does change sclite's behavior for some situations for CTM2CTM scoring. The particular case is (but luck) present in Test13.

Thanks both for contributing to SCTK!