-
Notifications
You must be signed in to change notification settings - Fork 14
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
Weights are modifying the observation SD not variance #25
Comments
Thanks for catching this. It's an error, because I meant to have the weights affecting the variance, not SD. Before making any changes, probably worth thinking about if this is the approach we want to use, or whether we should adopt the I can really go either way. I was thinking of this classical way initially because surveys brought into a DFA each might have an associated CV, etc. Using those as weights works with the classical implementation when likelihood is normal/lognormal, but the |
The current implementation has been useful so far for @ecophilina 's work modelling body condition indices. It sounds like brms takes the likelihood multiplier route too. Perhaps weights should multiply the likelihood but what you currently have could be something like an input observation variance? I.e., give it a different argument name. The current implementation seems more akin to what goes into meta-analysis, which seems like a common application. If changing, I guess there would have to be a warning for a while... although given it's already not implemented exactly how it's described, a change needs to be made regardless. |
@ecophilina -- see the changes I was thinking about here: #27 . Let me know if that's not clear or if there's another approach you were thinking of |
Looking at the implementation, it seems these are modifying the SD not the variance, right? I.e., if so, the argument should change name to inverse SD or the implementation should change to modify the variance not the SD. |
I'm not sure if this is just a naming issue, or something with the code that needs changing. If it's naming, we need to have 2 vectors -- one for optional likelihood weights, and one for optional inverse variance weights. Happy to change the arg name to something else. If it's a coding issue, I think where we want to get to is that if someone has standard errors of a survey ( The workflow that's currently implemented is that
|
Ah, I missed this line
inv_var_weights_vec = sqrt(1.0/inv_var_weights_vec),
It’s all fine then. I was confused since the vector name in the Stan code
makes it sound like it’s still the inverse variance, not the inverse SD.
|
SD/weight in the code:
bayesdfa/inst/stan/dfa.stan
Line 625 in 4d91db1
Variance/weight in the vignette:
bayesdfa/vignettes/a1_bayesdfa.Rmd
Line 325 in 4d91db1
I suppose adjusting the SD is fine and maybe the docs just need to be fixed? Or would scaling by inverse variance be more classical?
?fit_dfa
alludes to the implementation:bayesdfa/R/fit_dfa.R
Lines 82 to 84 in 4d91db1
The text was updated successfully, but these errors were encountered: