Skip to content
This repository has been archived by the owner on Jul 19, 2018. It is now read-only.

Global r2 #18

Merged
merged 4 commits into from
Feb 22, 2018
Merged

Global r2 #18

merged 4 commits into from
Feb 22, 2018

Conversation

TaylorOshan
Copy link
Collaborator

The following PR adds method to provide global R2 value to evaluate Gaussian GWR compared to OLS global model and satisfies the suggestion put forth in #7.

Copy link
Member

@ljwolf ljwolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing major, just a question about the reshapes.

@cache_readonly
def R2(self):
if isinstance(self.family, Gaussian):
TSS = np.sum((self.y.reshape((-1,1)) - np.mean(self.y.reshape((-1,1))))**2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we doing the reshapes? If they're just (n,), then (n,) - (n,) should be (n,). We'd need to do a reshape if we were trying (n,).T.dot((n,)) ,but we're just doing (n,)**2.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.predy is (n,1) so I think I had done it to make them all (n,1) to the same answer. Could just set predy to (n,) instead of reshaping all of them.

TSS = np.sum((self.y.reshape((-1,1)) - np.mean(self.y.reshape((-1,1))))**2)
RSS = np.sum((self.y.reshape((-1,1)) -
self.predy.reshape((-1,1)))**2)
return 1 - (RSS / TSS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps adding an else statement for non-Gaussian models?
Like for localR2

else:
    raise NotImplementedError('Only applicable to Gaussian')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right now, this would only return None in that case. good catch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, good call. Will now raise an error.

@Ziqi-Li Ziqi-Li merged commit a0b7f1a into pysal:master Feb 22, 2018
@Ziqi-Li Ziqi-Li mentioned this pull request Feb 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants