Numerical stability issues in floating point comparisons (chord directional hamming)

[bmcfee]

I ran into a funny bug just now, and wanted to get some opinions on whether this should be considered an mir_eval problem or not.

This check in the directional hamming distance function looks for disjoint time intervals by comparing the end of one interval to the start of the next.

This is fine in principle, but I have a few (reference) files which are stored in JSON (jams) format, and for whatever reason, deserializing them messes up the floating point precision just right so that it triggers an error. For example:

In [4]: J = jams.load('/home/bmcfee/data/crema/chords/TRAIIEF149E3861F6C.jams')                                                                                                                                    

In [5]: ints, labs = J.annotations[0].to_interval_values()                                                                                                                                                         

In [6]: ints[1, 1]                                                                                                                                                                                                 
Out[6]: 2.126712

In [7]: ints[2, 0]                                                                                                                                                                                                 
Out[7]: 2.1267119990000003

On the one hand, this is clearly a data error, and it's reasonable for mir_eval to fail here.

On the other hand, this is not a serious data error, and it's one that we can expect to pop up any time we're dealing with numerical data through text serialization, which is basically every use case of mir_eval.

Given that, what do people think of rounding the time intervals (say to the millisecond level) prior to checking? That is, just add lines to the effect of

reference_intervals = np.around(reference_intervals, decimals=3)

before the overlap check. For my money, I see no downside to this, but I'd like to get some opinions before opening a PR.

[craffel]

Does this cause an error or violated assumption later on, or do things work out? In other words, should we round solely for the check, or should we round any input to the corresponding metric functions?

I feel a little odd rounding user data, though we already sort of do it elsewhere in the library. It feels like it would make more sense to round in io and then if someone uses their own IO we let them deal with any resulting weirdness, but I do not feel strongly about this since it probably won't make a big difference and is probably what the user wants.

[bmcfee]

Does this cause an error or violated assumption later on, or do things work out?

It depends on the metric. Many of the metrics convert (labeled) intervals to samples at some fixed rate (eg 10Hz), in which case, rounding to the millisecond will not change the results.

Some of the metrics (directional hamming) use precise differences rather than sampling, and in this case, there should be negligible difference if we round the results.

The places where it could get dicey are in something like onset detection or pitch detection, where the tolerances are already quite small (5ms), and maybe a lower tolerance would be appropriate there (1e-4, for instance).

Because it depends on the metric / task, I'm hesitant to recommend rolling the rounding into io, which happens upstream of the metrics.

[craffel]

Sorry, in that question I was referring specifically to the check in chord, not any of the other tasks. Should the chord metrics have their intervals rounded just for the check, or for the metric function itself?

[bmcfee]

Sorry, in that question I was referring specifically to the check in chord, not any of the other tasks. Should the chord metrics have their intervals rounded just for the check, or for the metric function itself?

I think it should be for both.

[craffel]

Because if we don't round it things break? Or for consistency and because it's probably the right thing to do? Or what?

[bmcfee]

Or for consistency and because it's probably the right thing to do? Or what?

Yeah, sorry -- if we round the timing to validate it, we should compute on the rounded data because it's been validated.

[craffel]

Ok, do you think we should only fix this in chord or are there other places we should round annotations?