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

Fix SLX indirect impacts #26

Closed
wants to merge 2 commits into from
Closed

Fix SLX indirect impacts #26

wants to merge 2 commits into from

Conversation

nk027
Copy link
Contributor

@nk027 nk027 commented Dec 17, 2021

Hey,

So this PR closes issue #23 by scaling the coefficients of the linear model. I think this is ultimately the least invasive change for users, also see the commit message:

As documented in issue #23 and doi.org/10.13140/RG.2.2.11269.68328
the average indirect effects of the SLX model are not correct for
non-row-stochastic matrices. This commit fixes that.

To be maximally non-invasive to users, I apply the necessary scale
factor to the coefficients of the underlying LM model. This means
that the coefficients can be directly interpreted as partial
effects -- which is probably the way it's being done anyways.

This could also be addressed in a more involved manner (for users
and coders) -- we could apply the scale factor only to the indirect
impacts (returned by 'impacts()') and only adjust the model used to
compute total effects. I did not pursue this as it (1) would require
more code, (2) be more contrived, and (3) does not reflect usage
patterns that I would expect.

Also sorry for the messy changelog -- I should have separated actual changes from indentation adjustments better. The gist is this adjustment to the model:

# NK: Scale the impacts for proper average impacts if necessary (see #23)
if(listw$style != "W") { # For total impacts we need to apply this to the coefficents
      # Option A -- we scale the coefficients so they give average effects
      s_theta <- sum(vapply(listw$weights, sum, numeric(1L))) / n
      lm.model$coefficients[-seq(dvars[1])] <-
          lm.model$coefficients[-seq(dvars[1])] * s_theta
      # Option B -- we build a scale factor and apply that to indirect impacts + total ones
} # Row-stochastic has the proper scaling already

Nikolas Kuschnig added 2 commits December 17, 2021 22:59
As documented in issue r-spatial#23 and doi.org/10.13140/RG.2.2.11269.68328
the average indirect effects of the SLX model are not correct for
non-row-stochastic matrices. This commit fixes that.

To be maximally non-invasive to users, I apply the necessary scale
factor to the coefficients of the underlying LM model. This means
that the coefficients *can* be directly interpreted as partial
effects -- which is probably the way it's being done anyways.

This could also be addressed in a more involved manner (for users
and coders) -- we could apply the scale factor only to the indirect
impacts (returned by 'impacts()') and only adjust the model used to
compute total effects. I did not pursue this as it (1) would require
more code, (2) be more contrived, and (3) does not reflect usage
patterns that I would expect.
@nk027
Copy link
Contributor Author

nk027 commented Dec 17, 2021

Something to consider is how to handle this for SDM models -- it might also make sense to scale the coefficients there, but users should use the impacts() function for interpretation anyways.

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

Successfully merging this pull request may close these issues.

2 participants