Conversation
|
looks good to me, |
|
This looks good. Two quick questions, (1) Math is a "generic" Python library, correct? (2) The large number of text files is required because you are testing many of the cases presented with the original toolbox? |
|
Sorry for the late reply. Yes, Math is a library native to python. The large number of text files is essentially because i ran the entire simulation given in the demoscript, and verified correctness to the same degree for all of them. |
|
Note that kernel density estimates are already implemented in SciPy, so you probably want to re-use those. Also, please use the numpy documentation format to document functions. |
| from math import log | ||
|
|
||
|
|
||
| def kerneldensityestimation(source, target, timelagx, timelagy, n, bw_coeff): |
There was a problem hiding this comment.
According to PEP8, function names should be in the form kernel_density_estimation. That also goes for timelag_x and timelag_y.
|
Alright in light of the issues thus far (lack of testing coverage and the existence of KDE in scipy), I'll take some time after finals to re-write this function after understanding the math behind it. @stefanv should I close this pull request and make a new one later? |
|
You could do that, or you can simply keep pushing to this branch. In the
end, you can flatten / rewrite your commits. I'm happy to chat about
vectorization sometime if you'd like.
|
4 participants
matlab transferEntropyKDE and mdKDE functions are done, tests written and passing on local machine.