search

pollen-robotics / dtw

DTW (Dynamic Time Warping) python module
GNU General Public License v3.0
1.16k stars 233 forks source link

I want to know why return D1[-1, -1] / sum(D1.shape) in dtw function? #38

Closed iWangLin closed 5 years ago

iWangLin commented 5 years ago

why not return D1[-1, -1] ?

pierre-rouanet commented 5 years ago

Both can be used. I've used the normalised version which works better in high dimension.

marcus-voss commented 5 years ago

Hey Pierre, so this actually also confused me as it is not consistent with another Python implementation I used. This actually led me to try all the implementations I could find, also on your minimal example: https://gist.github.com/marcus-voss/bd53f6b71ca5e9c4d0c7cae60b9486d9

TL;DR, here the results:

fastdtw package (the not approximate version): 2.000, sqrt(d): 1.414
dtw package: 0.111, sqrt(d * (acc_cost_matrix.shape)): 1.414
Distance package of DTAI at KU Leuven: 1.414
tslearn package: 1.414
Implementation in Petitjean's official DBA code: 1.414

So while some return the squared DTW, none does this normalization as you do it. So I recommend to at least document it accordingly to avoid confusion. Note, that the sktime implementation did also work on other examples.

pierre-rouanet commented 5 years ago

Hi @marcus-voss and thanks for your investigation!

I've removed this normalisation in https://github.com/pierre-rouanet/dtw/commit/a02578bdf5a5024eca9e4d7eb78d97d6c8bb78ff as it seems indeed unusual and can be done afterward by users easily anyways!

I'll thus close the issue, do not hesitate to re-open it or open PR! I don't use this package anymore so it may be outdated but I'll gladly integrate new bug fix or features!

Cheers, Pierre

vladh commented 5 years ago

Hi @pierre-rouanet, just so you know, it doesn't seem like this change was published to pip. Would be great to do so, so that others are not confused about this issue. Thank you for the package! :)

pierre-rouanet commented 5 years ago

Done, thank you for noticing!