Consolidate MGWR #5

TaylorOshan · 2018-03-14T22:30:20Z

This PR:
(1) Merges together several copies of MGWR that were beginning to diverge from each other.
(2) Merges in several updates to the underlying GWR routines to the MGWR codebase
(3) Adds additional code to estimate a hat matrix in parallel with estimating the MGWR parameters.
(4) Adds some basic tests for MGWR (not including the hat matrix)

It would be good for everyone to a close look at this and make sure your favorite edits to the MGWR code are still here. I did my best to make sure everything was going in, but could have accidentally deleted something.

This should be seen as the starting point for the MGWR code that will underly the GUI and be released to the public, rather than a finished product. Still need to clean code up, add diagnostics, and more tests.

comment out ac in gs

Speed-up GWR

add scipy optimization and some exploratory demonstrations.

Solving divided by zero issue

TaylorOshan · 2018-03-14T22:37:20Z

It should be noted, that the MGWR routine we used for the GA paper had changed the initial bandwidth parameterization so that line 331 in sel_bw.py is if a > self.bw_min: rather than if a < self.bw_min: as it currently is in this PR (and the public GWR repo). Changing it breaks some of the pre-existing tests, which take a manual BW. I think the tests were based on results from GWR4, so I think this is one of the scenarios where we will need to break from GWR4, which I think has it wrong (at least in some circumstances) on how to specify what the minimum BW search space should be. Just be advised for ongoing research that you may need to change line 331 to if a > self.bw_min: if you want to effectively be able to manually set a very small minimum bandwidth.

Ziqi-Li

good job! good to me.

weikang9009 · 2018-03-15T00:21:30Z

gwr/sel_bw.py

+    def _mbw(self):
+        y = self.y
+        if self.constant:
+          X = USER.check_constant(self.X_loc)


indentation is not consistent.

Should be consistent now.

weikang9009 · 2018-03-15T00:31:05Z

gwr/sel_bw.py

+        bw_max = self.bw_max
+        interval = self.interval
+        tol = self.tol
+        max_iter = self.max_iter


I wonder whether there is a need to define new variables since they are not transformed in any way.

probably not, but I am going to leave them for now for the sake of getting a canonical version of MGWR set up - any PR's cleaning it up are welcome!

weikang9009 · 2018-03-15T00:44:07Z

gwr/search.py

+        err = optim_model.resid_response.reshape((-1,1))
+        est = optim_model.params
+    else:
+        model = GLM(y, X, family=self.family, constant=False).fit()


There is no self.family.

Right, it should just be family=family.

Its now just family. Should we think about taking out the GLM initialization?

weikang9009 · 2018-03-15T01:04:23Z

The interpretation of the optimal bandwidths produced from MGWR calibration depends on the magnitude of the independent variables (both mean and variance). A suggestion has been made in Note 5 of the MGWR paper (AAAG) to standardize them before calibrating an MGWR model to facilitate interpretation in terms of linking the values of the optimal bandwidths to the spatial scales of conditional relationships. I am just wondering if we are going to add some optional parameters to standardize X.

TaylorOshan · 2018-03-15T01:06:14Z

Yup, that's the plan! Been doing it manually, but will be helpful to have an option hat does it automatically.

…

On Wed, Mar 14, 2018 at 6:04 PM Wei Kang ***@***.***> wrote: The interpretation of the optimal bandwidths produced from MGWR calibration depends on the magnitude of the independent variables (both mean and variance). A suggestion has been made in Note 5 of the MGWR paper (AAAG) to standardize them before calibrating an MGWR model to facilitate interpretation in terms of linking the values of the optimal bandwidths to the spatial scales of conditional relationships. I am just wondering if we are going to add some optional parameters to standardize X. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#5 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHvdzZ3IjRi5X1n29_tQsDmQzg770is6ks5teb4XgaJpZM4SrRpF> .

ljwolf · 2018-03-15T01:17:17Z

While an option for mean/variance standardization important, I think we would definitely need to make sure a "summary" function or the MGWR __repr__disclaims that this has been done... I believe users see those betas and assume they're in the units of their input unless they've standardized it themselves.

I wrestled with this same question in spvcm and decided to not standardize by default after shooting myself in the foot a few times and handing the gun to others for a few shots as well.

ljwolf

minor changes/tiny questions.

ljwolf · 2018-03-15T01:17:43Z

gwr/diagnostics.py

@@ -1,5 +1,5 @@
 """
-Diagnostics for estimated gwr models
+Diagnostics for estimated gwr modesl


Good catch, fixed it up.

ljwolf · 2018-03-15T01:19:29Z

gwr/search.py

+        err = optim_model.resid_response.reshape((-1,1))
+        est = optim_model.params
+    else:
+        model = GLM(y, X, family=self.family, constant=False).fit()


Right, it should just be family=family.

ljwolf · 2018-03-15T01:22:27Z

gwr/gwr.py

    @cache_readonly
    def predictions(self):
        P = self.model.P
        if P is None:
-            raise NotImplementedError('predictions only avaialble if predict'
-            'method called on GWR model')
+          raise NotImplementedError('predictions only avaialble if predict'


Should this be a TypeError or Exception? feels weird to call it a NotImplementedError, since it's only possible to predict if you've fit.

Sure, TypeError makes a bit more sense. Might be a better way to implement in this in the future that avoids an ambiguous error. But just want to make sure the user gets some sort of message that indicates that GWRResults will only provide predictions if you've run gwr_model.predict() prior.

weikang9009 · 2018-03-15T01:26:23Z

I agree. The summary output for MGWR definitely needs to incorporate such standardization if it has been made (either it is made by default or not). Even if the users choose not to standardize, I think it is still important to remind them of a different interpretation of the optimal bandwidths in the summary output.

Ziqi-Li · 2018-03-15T01:30:54Z

@weikang9009 that can be easily done in the GUI's summary output, but for the console, we don't currently have a summary output unless do something like what statsmodel does. I know GWmodel has a text summary output to the R console after finish running.

Ziqi-Li · 2018-03-15T01:32:19Z

Perhaps can just do like print ("X and Y are standardized.") to the console if the standardization option is checked?

weikang9009 · 2018-03-16T16:06:46Z

+1. A summary output similar to spregmight be useful.

TaylorOshan · 2018-03-21T22:32:57Z

I've migrated the conversation about standardization to #6 where we can keep discussing, but I was hoping not to hamstring this PR on that issue. Also fixed up a bunch of minor revisions suggest over review, so I think this is ready to merge and become the up-to-date version of gwr/mgwr.

TaylorOshan · 2018-03-21T22:49:35Z

I also added a PR so that if a user manually inputs min or max bandwidth search parameters, then those are automatically set to the starting points of the search procedure. As it currently stands the default min/max are both zero and then this value is compared to the "rule of thumb" min/max and depending on if the manual min is lower than the standard min then the manual min becomes the min. However, this causes issues depending on whether the use wants to set a manual bandwidth that is smaller/larger than the standard min. So now the default is None and if they set min and/or the max then that will definitively become the search min/max.

Ziqi-Li and others added 30 commits December 7, 2017 14:33

comment out ac in gs

08b320c

Merge pull request #1 from Ziqi-Li/dev

1a63ae0

comment out ac in gs

pull from origin

3217ade

move fast search function from GWR class to Sel_BW class

055dfb0

move fast search function from GWR class to Sel_BW class

70631d6

clean up a bit

592857a

clean up a bit

4401b76

using iwls._compute_betas_gwr to fit

acd9e68

minor cleanup

31d3424

clean up

dd15f20

clean pyc

f0d5346

clean up

409e562

removing build_W_fast

7dfad4f

remove four comments in search.py

1e16be6

Merge pull request #10 from Ziqi-Li/dev2

dd7a5b9

Speed-up GWR

add R2 attribute

8405e5a

add scipy option to bandwidth optimization

21479e3

switch to "search_method" to avoid overwriting search method

a82cbde

add new bandwidth search functions

8fbfd9b

fix R2

75d17f4

add R2 test for BS NN scenario

2539b0f

Merge pull request #17 from ljwolf/scipy_opt

53db5a6

add scipy optimization and some exploratory demonstrations.

fetched from master

c8ab126

notebook

639e657

clean

018975e

remove comments

c244b38

rm pyc

00b2a44

update

45df985

clean

2b4a887

notebook explorations

7fdf993

TaylorOshan added 9 commits March 14, 2018 12:05

merge public gwr into private mgwr

56c75a4

update imports

da28950

add MGWR tests

b1c3bac

Merge pull request #21 from Ziqi-Li/div_by_zero

7457f32

Solving divided by zero issue

merge divide-by-zero solution

d4b2291

fix min_bw manual setting

198c0fa

add second method for hat matrix

653a0cd

more hat matrix edits

5f7d360

change manual min_bw back

e7d1e52

TaylorOshan requested review from ljwolf and Ziqi-Li March 14, 2018 22:38

Ziqi-Li approved these changes Mar 14, 2018

View reviewed changes

weikang9009 reviewed Mar 15, 2018

View reviewed changes

ljwolf requested changes Mar 15, 2018

View reviewed changes

TaylorOshan added 4 commits March 20, 2018 15:34

merge spat var test/GWR Lite/spherical; pull spherical into MGWR

5a43709

add SE notebook and pull through compoenet-wise vals

6ec0f1d

Clean up PR based on reveiw

163d0f1

change prediction error type

c0dca97

add manual bandwidth range fix

13e0f82

TaylorOshan merged commit 8665c8c into pysal:master Apr 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consolidate MGWR #5

Consolidate MGWR #5

TaylorOshan commented Mar 14, 2018

TaylorOshan commented Mar 14, 2018

Ziqi-Li left a comment

weikang9009 Mar 15, 2018

TaylorOshan Mar 21, 2018

weikang9009 Mar 15, 2018

TaylorOshan Mar 21, 2018

weikang9009 Mar 15, 2018

ljwolf Mar 15, 2018

TaylorOshan Mar 21, 2018

weikang9009 commented Mar 15, 2018

TaylorOshan commented Mar 15, 2018 via email

ljwolf commented Mar 15, 2018

ljwolf left a comment

ljwolf Mar 15, 2018

TaylorOshan Mar 21, 2018

ljwolf Mar 15, 2018

ljwolf Mar 15, 2018

TaylorOshan Mar 21, 2018

weikang9009 commented Mar 15, 2018

Ziqi-Li commented Mar 15, 2018

Ziqi-Li commented Mar 15, 2018

weikang9009 commented Mar 16, 2018

TaylorOshan commented Mar 21, 2018

TaylorOshan commented Mar 21, 2018

Consolidate MGWR #5

Consolidate MGWR #5

Conversation

TaylorOshan commented Mar 14, 2018

TaylorOshan commented Mar 14, 2018

Ziqi-Li left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weikang9009 commented Mar 15, 2018

TaylorOshan commented Mar 15, 2018 via email

ljwolf commented Mar 15, 2018

ljwolf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weikang9009 commented Mar 15, 2018

Ziqi-Li commented Mar 15, 2018

Ziqi-Li commented Mar 15, 2018

weikang9009 commented Mar 16, 2018

TaylorOshan commented Mar 21, 2018

TaylorOshan commented Mar 21, 2018