Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect behavior for Moran_Local and self-neighbors #86

Closed
ljwolf opened this issue Aug 12, 2019 · 5 comments · Fixed by #133
Closed

Incorrect behavior for Moran_Local and self-neighbors #86

ljwolf opened this issue Aug 12, 2019 · 5 comments · Fixed by #133

Comments

@ljwolf
Copy link
Member

ljwolf commented Aug 12, 2019

Moran_Local assumes no self-neighbors when running conditional permutations, but neither checks nor appropriately removes them.

This happens when we do conditional permutation, so I think it means that there's some issue with trimming the weights w[i] in Line 931. Basically, you end up getting one more observation than you want in w[i]

Suggested fix:

force remove all self-weighting when conducting any kind of conditional randomization in esda.

To reproduce:

import pysal, numpy
adjacency = numpy.array([[1,1,0],
                         [1,1,1],
                         [0,1,1]])
data = [1,2,3]
pysal.explore.esda.Moran_Local(data, adjacency)
@jeffcsauer
Copy link
Collaborator

This is what I am getting when I attempt to reproduce, but after experimenting a bit I'm not sure it's the same scenario you are getting. Is this the 'correct' incorrect output?

>>> import pysal, numpy
>>> adjacency = numpy.array([[1,1,0],[1,1,1],[0,1,1]])
>>> data = [1,2,3]
>>> pysal.explore.esda.Moran_Local(data, adjacency)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\jeffe\AppData\Local\Programs\Python\Python38-32\lib\site-packages\esda\moran.py", line 863, in __init__
    w.transform = transformation
AttributeError: 'numpy.ndarray' object has no attribute 'transform'

@ljwolf
Copy link
Member Author

ljwolf commented Mar 25, 2020

Hey @jeffcsauer, yeah that's my fault for missing a step in the reproducible example!

Sorry!

This should be it.

from pysal.lib import weights
from pysal.explore import esda
from scipy import sparse

adjacency = sparse.csc_matrix([[1,1,0],
                               [1,1,1],
                               [0,1,1]])
w = weights.WSP(adjacency).to_W()
data = [1,2,3]

esda.Moran_Local(data, w)

@jeffcsauer
Copy link
Collaborator

@ljwolf no worries at all! Thanks for the quick follow-up and clarification 👍

@sjsrey
Copy link
Member

sjsrey commented Apr 18, 2020

To clarify, is the use case actually requiring self-neighbors?
Or, is this a request to add error-checking to rule out self-neighbors?

@ljwolf
Copy link
Member Author

ljwolf commented Jul 10, 2020

@sjsrey this is further discussed over in #133. The crand engine makes this a particularly good time to deal with this, since we need to handle self-neighbors for Gi-star & LOSH

I propose (like Gi-star), we allow self-neighbors in local statistics and correctly handle their conditional randomization: hold their values constant and shuffle non-diagonal elements.

If we want to warn/exception on users passing self-weighted W in Moran_Local, we also need to do that in:

  • all Moran* statistics
  • Geary
  • Lee-type statistics
  • geosilhouettes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants