esda
esda copied to clipboard
pysal
Issues with assuncao_rate function and NaN
Hi!
I haven't worked this into test case data that would be shareable yet, but am using pysal.esda.moran.Moran_Rate in some code and running into nan output.
Debugging the code, it appears that the assuncao_rate smoother function does not include a check for negative values of ebi_v, which then leads to NaN values in the EB-adjusted rates values which propagate through all the calculations.
Assunçao's original paper says that, by convention, if ebi_v[i] < 0, then it should be set to ebi_v[i] = ebi_b / b[i], rather than ebi_a + ebi_b / b[i]. Am I wrong in thinking this should also be in the PySAL code? I have implemented the fix locally and happy to submit a PR.
@timstallmann, could you please link me to the paper you're referring to and provide some pointers towards implementing a fix like the one you have implemented? I've made a pull request to fix this.
Sure! Here's the article http://onlinelibrary.wiley.com/doi/10.1002/(SICI)1097-0258(19990830)18:16%3C2147::AID-SIM179%3E3.0.CO;2-I/full
Just reviewed your PR for fixing this and added a comment...
I've gotten an email notification from @jonnyhuck about this, but for some reason the comment isn't appearing on the package itself. Yes, the fix would easily be incorporated and I think is a correct read of the source paper (p. 2157 para 2)!
Hi @ljwolf - apologies - I had removed the comment as I did some more digging and found that it was a slightly different issue in the same assuncao_rate function, caused by one instance of my effect and population values both being 0. However, if you are going to edit the function this might be a good opportunity for raising a ValueError or similar - this situation is not caught by the suggested fix (i tried it). I was going to replace the comment with a better one once I had figured out the details!
I can provide details of exactly where the issue occurs (would just have to work it out again...), or propose a pull for that case if you like?
Details are helpful, and a PR would be awesome! Happy to review & integrate any changes.
What a blast from the past -- This appears to have been addressed when https://github.com/pysal/pysal/pull/931 was merged!
That is odd - it isn't reflected here: https://github.com/pysal/esda/blob/master/esda/smoothing.py#L563
This might be down to my understanding of the interaction of the modules - but it certainly isn't reflected in my version from Anaconda either.
Indeed, I'm not sure if there's been an issue with the migration of the federating of pysal (where we migrated this stuff from pysal to the esda package) but the fix should be made here now.
This is odd.
Happy to merge the fix and do a bugfix release asap.