Closed NoPenguinsLand closed 10 months ago
I am really happy to see contributions to the project but this is a rather large change to the GLM function, which is already bloated in functionality. Is it possible to add this additional functionality as another user function?
Hey Stephen, happy to help! I added multiple functionalities, do you mean to break them down in separate functions (which is rather hard to separate them) or just rename the function (maybe hmrR_GLM_Extended)?
I am not familiar with what you're doing here, but ideally more sophisticated analysis can be modularized and made into a function which operates on the outputs of GLM... Maybe this is totally impossible in this scenario? @mayucel
It's possible but all of these functionalities are intimately tied to GLM calculations so I personally think it's best to leave it as is, plus it will run faster when put together. Sure, let's wait and hear what Meryem has to say.
Thanks Stephen!
On Mon, May 16, 2022, 7:51 AM stephen scott tucker @.***> wrote:
I am not familiar with what you're doing here, but ideally more sophisticated analysis can be modularized and made into a function which operates on the outputs of GLM... Maybe this is totally impossible in this scenario? @mayucel https://github.com/mayucel
— Reply to this email directly, view it on GitHub https://github.com/BUNPC/Homer3/pull/131#issuecomment-1127573991, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRZ6NUMLLMXDNQYU4FISADVKIZFJANCNFSM5TIWD65Q . You are receiving this because you authored the thread.Message ID: @.***>
Testing: I tested all 4 different basis functions (idxBasis) and 2 different glmSolveMethod (GLM or iterative WGLM AR(1)). I also tested it for both t-test and F-test. So I think it should work under most reasonable scenarios. Another thing to note, technically, you can use it for both t-test and reduced model F-test but I don't think using p-value from reduced model makes sense. Any extraordinary scenarios, like users abusing Homer3, I haven't really checked these.
Changes:
Considerations: