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

Sign and significance is sorting by the wrong quantity #20

Closed
rgiordan opened this issue Aug 3, 2021 · 3 comments
Closed

Sign and significance is sorting by the wrong quantity #20

rgiordan opened this issue Aug 3, 2021 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@rgiordan
Copy link
Owner

rgiordan commented Aug 3, 2021

@tinnguyen96 writes:

Based on lines 135–138 from regression_sensitivity_lib.R, to induce a “sign and significance” change in a negative, statistically insignificant estimate, it is sufficient to change the value of the “beta mzse est” function (posterior mean minus two posterior standard deviations). Furthermore, the direction of the change should be “pos”, since we need to increase the value of “beta mzse est”. Conceptually, we need to select observations with the smallest values of “beta mzse grad”.

In the implementation, for this situation, GetRegressionTargetChange() actually select ob-
servations with the smallest values of “beta pzse grad”. Based on lines 166 and 145 from regression_sensitivity_lib.R, if influence dfs is the output of SortAndAccumulate(), then GetRegressionTargetChange() will use influence dfs$sig$pos to select the observations. However, the observations ranking in influence dfs$sig$pos is based on increasing beta pzse grad rather than increasing beta mzse grad. For evidence of this, look at line 139 from sorting_lib.R.

@rgiordan rgiordan self-assigned this Aug 3, 2021
@rgiordan
Copy link
Owner Author

rgiordan commented Aug 3, 2021

I think the problem comes down to organizing the influence scores by "sign" and "significance." In hindsight, this was quite a bad design decision. My first instinct is to re-design so that the influence scores list contains named entries for each sorted metric, which, by default, would be "beta", "beta_mzse", and "beta_pzse". We can then get rid of the "pos" and "neg" elements and just use the first or last entries of the list instead.

@rgiordan rgiordan added the bug Something isn't working label Sep 10, 2021
@rgiordan
Copy link
Owner Author

A solution (and major refactor) is in progress on the refactor branch.

@rgiordan
Copy link
Owner Author

rgiordan commented Oct 2, 2021

Fixed in #22

@rgiordan rgiordan closed this as completed Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant