Skip to content

280 full ddm#302

Open
GidonFrischkorn wants to merge 37 commits intodevelopfrom
280-full-ddm
Open

280 full ddm#302
GidonFrischkorn wants to merge 37 commits intodevelopfrom
280-full-ddm

Conversation

@GidonFrischkorn
Copy link
Collaborator

Summary

  • Implements the ddm based on the likelihood provided by STAN.
  • This provides mostly a convenience implementation to simplify the use of the wiener function available in STAN and with our create_initfun the user no longer needs to worry about writing their own initial value function.
  • The 7par version uses a new likelihood function for STAN, but runs very slow. There is some documentation on this in the vignette
  • Distribution functions are based on the functions implemented in resists, this is similar to brms.

Tests

[x] Confirm that all tests passed
[x] Confirm that devtools::check() produces no errors

Release notes

@GidonFrischkorn GidonFrischkorn linked an issue Jan 24, 2026 that may be closed by this pull request
11 tasks
@GidonFrischkorn
Copy link
Collaborator Author

The R-CMD-checks fail, because the most recent STAN version is not available here on GitHub. For the 7-parameter wiener likelihood STAN 2.35 or newer is required. Given the feasibility issues with running the 7par version, we could also consider not exposing it.

Maybe, we can have a chat about that.

@venpopov
Copy link
Owner

venpopov commented Jan 24, 2026

Judging by the error message it is because the check on github is using rstan as the backend, and rstan is at least a year behind in shipping the STAN headers. This also means that it will fail on CRAN as well.

There are two options:

  • do not include the 7par model
  • include it but gate it and its test behind a check if 'cmdstanr" is available. As we do for example in an optional function calc_error_relative_to_nontargets, which checks if tidyr is available:
  stopif(
    !requireNamespace("tidyr", quietly = TRUE),
    'The "tidyr" package is required for this functionality'
  )

It's allowed on CRAN to have functions that depend on Suggested packages, but you have to check if they are available. In this case you would check if "cmdstanr" is available and whether the backend is specified as "cmdstanr". The latter is done by default if the package is installed, but users could in principle explicitely override it and request "rstan", in which case you would intentionally give an error message.

@GidonFrischkorn
Copy link
Collaborator Author

I have tried a few things to resolve the failing checks. So far none of them worked... Maybe the ddm also needs a specific version of cmdstanr so that we need to specify that this version is required for the model.

I will check this and get back to you, once I found a solution.

The alternative is to scrap the 7par version completely and basically provide a wrapper around the brms implementation.

@GidonFrischkorn
Copy link
Collaborator Author

After some consideration I decided to remove the 7par version and adjusted the functions accordingly. Now the checks all pass :-)

@GidonFrischkorn GidonFrischkorn marked this pull request as draft January 28, 2026 15:52
@GidonFrischkorn
Copy link
Collaborator Author

GidonFrischkorn commented Jan 28, 2026

I have decided to evaluate what it would take to implement the mixture between uniform and model likelihood for the rt models directly in STAN for the ddm given that the model is practically already available in brms and the implementation is mostly improving usability as no initial value function has to be written and the model formula is compiled for the user.

But it seems, that there is more to it, then just adding the mixture in the STAN likelihood. For now, there are some convergence issues. Thus, I switched this pull request to a draft.

@GidonFrischkorn GidonFrischkorn marked this pull request as ready for review January 29, 2026 20:31
@GidonFrischkorn
Copy link
Collaborator Author

The implementation of the mixture of uniform and ddm distribtuion is more complicated and will require some more comprehensive testing and evaluation of different implementation strategies. So far, I did not manage to find a combination between priors and likelihood specification that results in stable sampling and good model convergence.

I would prefer looking into this for a later release and for now keep the pre-processing with using ML mixtures implemented for #309 as a tool for identifying potential outlier RTs. This is something that work well already and provides an acceptable workflow from my perspective.

So, I re-open this pull request with the 3par and 4par version implemented.

@GidonFrischkorn GidonFrischkorn added this to the 1.3.0 milestone Feb 1, 2026
@GidonFrischkorn GidonFrischkorn added the enhancement - new model Request for new model label Feb 1, 2026
@GidonFrischkorn GidonFrischkorn self-assigned this Feb 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement - new model Request for new model

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[new-model] Implement full ddm

2 participants