Heteroscedastic error models - #31
Conversation
syiman
left a comment
syiman
left a comment
There was a problem hiding this comment.
Thoughts:
-
This PR seems to have introduced a seed for RNG, but some of functions in the samplers still use unseeded RNG functions. Maybe we should unify all RNG to one global state?
-
I don't really understand the variance floors in this. The homoscedastic sampler has a hardcoded floor of 1e-6, and the heteroscedastic sampler has a floor of 1e-9 in the error model file. As far as I understand though, the variance is kept positive by construction and sampling, so I don't understand the divergence in the logic.
-
Between the HoEM and HeEM parametrizations sigma and sigma2 is used inconsistently. It feels better to me if they both use either sigma or sigma2
Also, I'm not sure how feasible the change is for this next version, but right now, each coefficient sampled for the heteroscedastic error model is individually required to be positive. This would only affect really narrow cases, but maybe only the variance overall should be required to be positive? For example maybe sometimes the cross term should be negative and the other terms should be positive
syiman
left a comment
syiman
left a comment
There was a problem hiding this comment.
This all looks great to me! All the changes I wanted have been addressed. I believe it's ready to merge.
3 participants
The package is now updated to included heteroscedastic error models. The codebase, tests, docs, and other related things are updated to include HeEM.