-
Notifications
You must be signed in to change notification settings - Fork 3
Fixes & Test Refactoring #56
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
Open
FBartos
wants to merge
36
commits into
master
Choose a base branch
from
major-refactoring
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…itted models with pre-computed marginal likelihoods, centralize all model fitting, and add test organization guide (#52)
…d print testing (#53) * Initial plan * Update test-summary-tables.R to use prefitted models with file-based print testing - Created new test-summary-tables.R that uses prefitted models from test-00-model-fits.R - Implemented file-based print testing using saved reference files - Created generate_print_references.R script to generate reference output files - Added tests/results/print/ directory with reference files - Tests now cover multiple model types: simple priors, weightfunction, formulas, interactions, random effects, expressions - Note: Some model types in test-00-model-fits.R have issues with empty data lists that need separate fixing Co-authored-by: FBartos <[email protected]> * Add documentation and cleanup for test-summary-tables update - Added comprehensive README explaining changes and usage - Removed old backup file - Updated .gitignore to exclude backup files - Documented known issue with empty data lists in test-00-model-fits.R Co-authored-by: FBartos <[email protected]> * Address code review feedback: improve path handling and remove UPDATE_OUTPUT - Fixed .gitignore newline issue - Replaced read.table with readLines for simpler file reading - Use testthat::test_path for robust path resolution - Removed UPDATE_OUTPUT flag from test file (use generate_print_references.R instead) - Improved path handling in generate_print_references.R script - Updated README to reflect correct usage Co-authored-by: FBartos <[email protected]> * Fix consistency between file reading and writing methods - Changed generate_print_references.R to use writeLines instead of write.table - Regenerated reference files with consistent writeLines format - Updated README to show readLines in example (matching actual code) - Simplified path resolution in generate_print_references.R - Added clarifying comment about test_path() path construction Co-authored-by: FBartos <[email protected]> * Fix: use seq_along instead of 1:length for safer iteration - Changed loop in test-summary-tables.R to use seq_along(fits) to avoid issues with empty vectors - This follows R best practices and matches the pattern already used in generate_print_references.R Co-authored-by: FBartos <[email protected]> * Address PR feedback: consolidate tests and move generation to if(FALSE) block - Removed all class-only tests that didn't validate printed output - Consolidated all testing into single comprehensive print test - Moved generate_print_references.R content into test-summary-tables.R in if(FALSE) block - Expanded reference files from 6 to 16 to cover more table types - Tests now compare printed output for: weightfunction models, simple priors, transformations, and empty tables - Deleted standalone generate_print_references.R file Co-authored-by: FBartos <[email protected]> * Update README to reflect new test structure with if(FALSE) block - Documented that generation code is now in test file in if(FALSE) block - Updated instructions for regenerating reference files - Clarified that there are now 16 reference files covering more table types - Removed references to standalone generate_print_references.R script Co-authored-by: FBartos <[email protected]> * Add test and vignette organization instructions Added .github/instructions/tests.instructions.md and vignettes.instructions.md to provide guidelines for organizing tests and vignettes. Removed outdated TEST_ORGANIZATION.md and test-summary-tables-README.md. Refactored test-summary-tables.R to use a helper for reference file management, added configuration for reference file generation, and improved coverage for advanced table features and empty table cases. * finish summary tables --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: FBartos <[email protected]> Co-authored-by: František Bartoš <[email protected]>
* add more plots and tables * add `as_mixed_posteriors` tests for plots * Update test-JAGS-ensemble-tables.R * rename plot file * update test results * clean up formula and other files * clean marginal tests * update tests() * title naming
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors BayesTools tests for improved speed and organization while fixing two bugs: incorrect mixture prior ordering and formula handling for intercepts coded as 0.
Key Changes:
- Major test suite refactoring: centralized model fitting in
test-00-model-fits.R, extracted posterior processing helpers toR/posterior-extraction.R - Fixed mixture prior printing to maintain correct component ordering
- Extended formula parsing to handle
0as no-intercept indicator (previously only-1was supported)
Reviewed changes
Copilot reviewed 228 out of 849 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| R/summary-tables.R | Refactored runjags_estimates_table to use new extraction helpers, reducing ~150 lines of duplicated code |
| R/posterior-extraction.R | New file: Consolidated posterior extraction logic into reusable helper functions |
| R/priors-print.R | Fixed mixture prior component ordering bug |
| R/JAGS-formula.R | Extended .add_intercept_to_formula to handle 0 in addition to -1 |
| R/model-averaging.R | Fixed seed handling in mix_posteriors and as_mixed_posteriors |
| R/marginal-distributions.R | Added seed consistency for multi-level factor priors |
| R/JAGS-fit.R | Refactored JAGS_check_convergence to use new extraction helpers |
| R/JAGS-diagnostics.R | Simplified JAGS_diagnostics_autocorrelation using extraction helpers |
| R/priors-density.R | Fixed x-range calculation for spike-and-slab priors |
| tests/results/* | Added reference output files for test suite (generated artifacts) |
| .github/instructions/* | New files: AI agent instructions for vignettes and tests |
| DESCRIPTION | Version bump to 0.2.24, testthat edition 3 |
| NEWS.md | Documented changes |
| man/*.Rd | Generated documentation for new helper functions |
JAGS_formula() now supports '__default_factor' and '__default_continuous' entries in prior_list, allowing users to specify default priors for factor and continuous predictors not explicitly listed. Documentation and examples were updated, and comprehensive unit tests were added to verify correct behavior and error handling.
#57) * Initial plan * Implement automatic standardization of continuous predictors - Add formula_scale parameter to JAGS_formula function - Implement standardization logic for continuous predictors - Store mean and SD for back-transformation - Propagate formula_scale through JAGS_fit - Add transform_scale_samples function for posterior transformation - Add comprehensive tests for standardization Co-authored-by: FBartos <[email protected]> * Update NEWS.md with standardization feature documentation Co-authored-by: FBartos <[email protected]> * Improve unit tests for standardization feature - Add scaled version of interaction model to test-00-model-fits.R - Replace simple JAGS fitting test with comparison tests using pre-fitted models - Add visual comparison tests using vdiffr for manual assessment - Tests compare unscaled vs scaled models and verify transform_scale_samples() - All tests pass successfully Co-authored-by: FBartos <[email protected]> * Refactor tests to compare manual vs automatic scaling - Replace scaled vs unscaled comparison with manual vs automatic scaling - Create data with large scale differences (mean=1000, sd=500 vs mean=0.5, sd=0.1) - Fit two models with same seed: manual scaling and automatic scaling - Tests verify both approaches produce equivalent results - Visual comparisons show manual and automatic scaling produce identical posteriors - Remove old snapshot files, create new ones for manual vs auto comparison Co-authored-by: FBartos <[email protected]> * improve tests * Add scaling support to JAGS_evaluate_formula - Update JAGS_evaluate_formula to apply scaling when predictors were scaled during fitting - Check for formula_scale attribute in fit object - Automatically scale new data using stored mean/sd for each predictor - Add comprehensive tests comparing manual vs automatic scaling in predictions - Tests verify predictions match when using scaled data vs automatic scaling - All tests pass (119 total) Co-authored-by: FBartos <[email protected]> * Add formula_scale_list support to JAGS_bridgesampling - Update JAGS_bridgesampling to accept formula_scale_list parameter - Pass formula_scale to JAGS_formula calls during marginal likelihood computation - Ensures marginal likelihoods match between manual and automatic scaling - Add test verifying marginal likelihoods are equivalent - Update documentation - All tests pass (120 total) Co-authored-by: FBartos <[email protected]> * improve tests * fix input * catch changes * push to PC * Preserve attributes when unscaling posterior samples Ensures that class and attributes of posterior samples are preserved after applying the unscaling transformation in update.BayesTools_table. Updates tests to check correct unscaling of coefficients in the presence of interaction terms, including proper handling of main effects and interactions. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: FBartos <[email protected]> Co-authored-by: František Bartoš <[email protected]>
Introduces support for a 'log(intercept)' formula attribute in JAGS_formula(), JAGS_evaluate_formula(), and marginal likelihood computation. This enables model syntax of the form log(intercept) + sum(beta_i * x_i), useful for parameters constrained to be positive. Documentation and tests are updated to reflect and verify the new functionality.
Adds support for parameters with log(intercept) scaling by storing a log_intercept attribute in formula_scale, updating transform_treatment_samples to correctly transform intercepts, and renaming intercepts to exp(intercept) in format_parameter_names. Updates summary table functions and documentation to reflect these changes. Enhances tests to verify correct handling of log(intercept) scaling.
Introduces advanced filtering options to `runjags_estimates_table()` and related helpers, allowing removal or retention of parameters by name or formula via `remove_parameters`, `remove_formulas`, `keep_parameters`, and `keep_formulas` arguments. Updates documentation and tests to cover new filtering logic, and adds internal helpers for parameter filtering and removal.
Introduces a `probs` argument to `runjags_estimates_table()` and `runjags_estimates_empty_table()` for specifying custom quantiles, defaulting to c(0.025, 0.5, 0.975). Quantile column names in `runjags_estimates_table()` and `stan_estimates_table()` are changed from lCI/Median/uCI to their numeric values for consistency. Adds a `remove_diagnostics` argument to exclude MCMC diagnostics from output tables. Updates documentation, tests, and README to reflect these changes.
Enhances parameter filtering to automatically include PET, PEESE, and omega when 'bias' is specified in remove_parameters or keep_parameters, based on the bias prior type. Updates documentation and adds comprehensive tests to verify correct handling of bias-related parameters.
Refactored plotting functions to better handle PET, PEESE, PETPEESE, and weightfunction parameters, especially when present in the bias parameter as mixtures. Added .simplify_as_mixed_posterior_bias helper for extracting and simplifying bias posteriors. Updated as_mixed_posteriors to correctly set prior weights for conditional mixture posteriors. Expanded and improved tests for model averaging plots with complex JAGS models, including new vdiffr visual regression checks for PETPEESE and bias parameters.
Introduces the effect_direction argument to plot_posterior(), plot_prior_list(), lines_prior_list(), and geom_prior_list() for PET-PEESE regression plots, allowing users to specify 'positive' (default) or 'negative' effect direction. Updates documentation and internal logic to support this feature, and adds new test snapshots for the negative effect direction.
check from: SECTION: get_scale_transformation with plotting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Features
Fixes
0(instead of only-1)