Fix std bayes error#1193
Conversation
Summary of ChangesHello @mattansb, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue with the standardization of Bayesian models, aiming to harmonize their behavior with non-Bayesian models. The core changes involve restructuring the internal logic for applying standardization, particularly when the "refit" method is specified, and refining how standardized coefficients are processed. This ensures a more consistent and robust approach to parameter standardization across different model types. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the standardization of Bayesian models to align with the implementation for non-Bayesian models, which improves consistency across the package. The changes are generally well-structured and include corresponding updates to tests.
However, I've identified two high-severity issues in the new implementation. Both relate to the handling of inferential statistics (like Bayes Factors) after standardization. For the refit method, the logic to exclude these tests is incomplete. For post-hoc methods, invalid statistics from the unstandardized model are carried over to the standardized results. I've left specific comments with suggestions on how to address these correctness issues.
|
@strengejacke failure are only due to snapshots. |
|
Looks good to me! I'd say only two minor things are the linters you could address at lines 1108 and 1109. lintr line 1108 makes sense from the performance perspective ( |
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #1192 where model_parameters(standardize=) failed for Bayesian models with errors about differing row counts. The fix rewrites the standardization logic for Bayesian models to be more consistent with non-Bayesian models.
Changes:
- Added support for standardize parameter in
model_parameters()for Bayesian models - Implemented validation to prevent
test = "all"when standardizing (not supported) - Added filtering of scale-dependent inferential statistics (rope, bayes_factor) when standardizing
- Refactored code formatting across multiple files for consistency
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/testthat/test-standardize_parameters.R | Added comprehensive tests for Bayesian model standardization with various test statistics; improved code formatting |
| R/methods_coxme.R | Improved code formatting and consistent use of isTRUE() for standardize checks |
| R/extract_parameters.R | Core fix: Added standardization support for Bayesian models, validation logic for test parameters, and setting standardize to NULL after refit |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closes #1192
I basically re-wrote the standardization of Bayesian models to be more in-line with non-Bayesian model.