Open
Conversation
… DDM implementations
Replace the competing-risks approximation with actual DDM simulation using rtdists::rdiffusion(). This is theoretically consistent since the censored shifted Wald model is an approximation to the Wiener diffusion model for high-accuracy tasks. Changes: - Add rtdists to Imports in DESCRIPTION - Modify rcswald() to use rtdists::rdiffusion() with parameter mapping: drift -> v, bound -> a, ndt -> t0, zr*bound -> z, s -> s - Update documentation to describe the DDM-based generation approach - Export dcswald() and rcswald() in NAMESPACE Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Improve swald_lpdf and swald_lccdf in cswald_helper_functions.stan: swald_lpdf (Wald density): - Compute entirely in log-space to avoid overflow in intermediate steps - Pre-compute common terms (sigma_sq, log_t) to reduce redundant operations swald_lccdf (Survival function): - Replace Phi() + log1m() with std_normal_lccdf/lcdf + log_diff_exp - Keep scaling constant in log-space to prevent overflow with large drift/bound - Pre-compute sigma_sqrt_t to avoid redundant computation These changes ensure stable gradient computation for HMC sampling, especially when survival probabilities are very small (CDF close to 1). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Refactor both posterior_predict_cswald_simple and posterior_predict_cswald_crisk: - Extract parameters at start of function for cleaner code - Add explicit zr = 0.5 in simple version (was implicit default) - Document the bound * 2 transformation in simple version: converts single-boundary distance to DDM total boundary separation - Improve code style with consistent spacing - Retain negative_rt option for consistency with brms::wiener - Simplify return statements (remove intermediate assignment) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add log_lik_cswald_simple and log_lik_cswald_crisk functions to enable model comparison via loo(), waic(), and bridge sampling. Implementation details: - Extract parameters from posterior draws via brms::get_dpar() - Get observed RT and response from prep$data - Replicate observed data to match number of posterior draws (required because dcswald expects equal-length vectors) - Call dcswald() with appropriate version parameter This enables: - Leave-one-out cross-validation (loo) - WAIC computation - Bayes factor calculation via bridge sampling - K-fold cross-validation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes and improvements: - Fix incorrect "response variable" label for RT variable in error message - Fix incomplete error message for invalid response values - Fix typos: "then" -> "than", "the" -> "than", "inital" -> "initial" - Change "ddm" reference to "cswald" in error messages - Fix "numerical" type check to "integer" (R uses "integer" not "numerical") New validation features: - Add NA value checks for both RT and response variables - Add factor response variable handling (converts to character first) - Add warning for high error rates (>20%) in simple version - Add warning for small sample sizes (<50 observations) - Add NextMethod() call for proper method chaining Style improvements: - Consistent spacing around brackets and commas - Improved error message formatting with line breaks Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Documentation improvements: - Replace placeholder @param descriptions with detailed documentation - @param rt: Explain response times should be in seconds - @param response: Document all accepted formats (0/1, lower/upper, logical) - @param links: List available parameters for each version - @param version: Add itemized list explaining simple vs crisk versions - Add @Keywords bmmodel for proper indexing - Add @Seealso linking to dcswald() and rcswald() - Add complete working @examplesIf with data simulation and model fitting Bug fix: - Fix typo "boundary seperation" -> "boundary separation" in version table Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Implement cumulative distribution function (pcswald) and quantile function (qcswald) for the censored shifted Wald model. For the "simple" version, these functions are only defined for response=1 (correct responses) since errors are treated as censored observations. For the "crisk" version, both functions use rtdists::pdiffusion() and rtdists::qdiffusion() internally for accurate computation of the defective CDF and its inverse. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Create test-model_cswald.R with 70 tests covering: - Distribution functions (dcswald, pcswald, qcswald, rcswald) - Parameter validation and error handling - Model construction for simple and crisk versions - Data validation (check_data.cswald) including: - Required variables presence - NA value detection - RT validation (negative, large, small values) - Response format handling (integer, logical, character, factor) - Error rate warnings for simple version - Small sample size warnings - Formula conversion (bmf2bf.cswald) - Model configuration (configure_model) - Integration tests with mock backend Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move cswald distribution function tests (dcswald, pcswald, qcswald, rcswald) from test-model_cswald.R to test-distributions.R for consistency with other distribution function tests in the package. test-distributions.R now contains: - 14 cswald distribution function tests (previously in test-model_cswald.R) - All other distribution tests (sdm, mixture2p, mixture3p, imm, m3) test-model_cswald.R now contains only model-specific tests: - Model construction (5 tests) - Data validation/check_data (13 tests) - Formula conversion (1 test) - Model configuration (2 tests) - Integration tests with mock backend (3 tests) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Create comprehensive vignette (bmm_cswald.Rmd) covering: - Introduction and use cases for cswald model - Theoretical background: - Shifted Wald distribution - Censoring approach for error responses - Relationship to Wiener diffusion model - Two model versions (simple vs. competing risks) - Step-by-step fitting example with simulated data - Parameter interpretation (drift, boundary, ndt) - Guidance on when to use each version - Comparison table with brms::wiener Also add Miller et al. (2018) and Ratcliff & McKoon (2008) references to REFERENCES.bib. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove lower bound constraint for drift parameter (lb = NA instead of 0) - Update crisk priors: center drift at 0 with normal(0,1) prior - Update crisk init_ranges: drift now starts in [-0.5, 0.5] - Add test verifying negative drift generates more lower responses - Fix test-helpers-data.R to include rt/response for cswald models - Clean up vignette structure and fix bound transformation Negative drift allows modeling response biases at the accumulation level: positive drift = evidence toward upper boundary, negative = toward lower. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix typo: "boundary seperation" -> "boundary separation" - Fix typo: "relative startin point" -> "relative starting point" - Remove unused env_cswald_crisk variable in configure_model.cswald_crisk - Add comprehensive edge case tests: - Extreme drift values (high, low, negative) - Extreme bound values (small, large) - Boundary zr values (near 0 and 1) - All-correct responses scenario - High error rate data with crisk version Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add recycle_cswald_params() helper for consistent parameter validation - Validate that parameters are length 1 or length n, error otherwise - Replace hardcoded qcswald upper bound (100s) with adaptive calculation based on expected RT (bound/drift) - Add tests for: - Invalid parameter vector lengths - Consistency between simple and crisk versions - Adaptive bounds with slow drift rates Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
11 tasks
GidonFrischkorn
requested review from
venpopov
and removed request for
venpopov
Owner
|
When I try to run the main example, I get an error (even without my commit, which was only dealing with the distribution functions): can you debug it before I continue the review? |
Collaborator
Author
|
Just pushed a commit that resolves the issue. I thought that I can remove the initial values for mu, from the model specification. But for some reason the model still requires them. Maybe we can see, if we can use the |
Owner
|
Yeah I was wondering earlier about void my for something else, because we put it there for tracking but never use it. I will make it a general property to set init to the fixed value of mu when it’s void. Actually we should do that for all fixed parameters regardless
…On Feb 1, 2026 at 7:51 PM +0100, Gidon Frischkorn ***@***.***>, wrote:
GidonFrischkorn left a comment (venpopov/bmm#301)
Just pushed a commit that resolves the issue.
I thought that I can remove the initial values for mu, from the model specification. But for some reason the model still requires them.
Maybe we can see, if we can use the void_mu = TRUE setting to handle this internally.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because your review was requested.Message ID: ***@***.***>
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
cswaldtobmmimplementing the estimation of diffusion model parameters via the censored shifted Wald likelihoodcswaldcswaldTests
[x] Confirm that all tests passed
[x] Confirm that devtools::check() produces no errors
Release notes