search

pysal / spopt

Spatial Optimization
https://pysal.org/spopt/
BSD 3-Clause "New" or "Revised" License
304 stars 46 forks source link

bug: maxp implementation is not using contiguity #3

Closed sjsrey closed 3 years ago

sjsrey commented 5 years ago

The stub for https://github.com/pysal/spopt/blob/master/spopt/BCMaxP.py#L210 is using a general spatial distance rather than contiguity. We need to redesign the API to clarify what type of matrix is required for the different problems we will implement

renanxcortes commented 5 years ago

The stub for https://github.com/pysal/spopt/blob/master/spopt/BCMaxP.py#L210 is using a general spatial distance rather than contiguity. We need to redesign the API to clarify what type of matrix is required for the different problems we will implement

I see that this script tries to follow the original paper of 2012, but would fixing this bug be enough to guarantee the right implementation on maxp?

I know that the following question doesn't have a direct relation of this issue, but what is the substantial difference of these scripts to the ones present in https://github.com/pysal/region/tree/master/region/max_p_regions?

jGaboardi commented 5 years ago

Was this resolved by @renanxcortes's recent maxp fix?

sjsrey commented 5 years ago

Was this resolved by @renanxcortes's recent maxp fix?

No. the bug is specific to BCMaxP.py

OMahmoodi commented 4 years ago

I'm trying to implement a spatially-constrained clustering, but I cannot find region.Maxp or spopt (the new package). I'm using the latest version of Pysal. Would you please advise where I can find the regionalization tools? cheers,

renanxcortes commented 4 years ago

I'm trying to implement a spatially-constrained clustering, but I cannot find region.Maxp or spopt (the new package). I'm using the latest version of Pysal. Would you please advise where I can find the regionalization tools? cheers,

Hi @OmidMahmoodi , I think you could also take a look at this package: https://github.com/pysal/region. Although it will be deprecated in favor of spopt.

OMahmoodi commented 4 years ago

Thanks @renanxcortes . I'm using pysal v.2.1.0, and 'region' is no longer available in this version.

knaaptime commented 4 years ago

although region is being phased out in favor of spopt, you can still install it directly, with

pip install region or conda install -c conda-forge region

geosnap also wraps some of regions (and others') spatially-explicit clustering methods and might be of interest, depending on your application

jGaboardi commented 4 years ago

It seems I moved this issue to "Done." Is that the actual case or did I jump the gun?

renanxcortes commented 4 years ago

It seems I moved this issue to "Done." Is that the actual case or did I jump the gun?

I wouldn't say that this issue is "Done", since the spopt has a different API than region. The contiguity issue was fixed in region and I'm not really sure if that's the case in spopt. Isn't it @sjsrey ?

jGaboardi commented 4 years ago

@renanxcortes OK. I think I had just accidentally put it in the "Done" bin. I moved it back out.

knaaptime commented 3 years ago

i think this is resolved