Skip to content
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

Feature/108 implement sdt models #113

Draft
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

GidonFrischkorn
Copy link
Collaborator

@GidonFrischkorn GidonFrischkorn commented Feb 21, 2024

Summary

  • Add SDT models for old/new (signal/noise) recognition as a bmmmodel class.
  • Add distribution functions for density, and random generation for SDT models
  • Initiate aggregate data method (experimental) for aggregating data to speed up model

Tests

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

Release notes

@GidonFrischkorn GidonFrischkorn added the PR - minor Pull-request should update minor version label Feb 21, 2024
@GidonFrischkorn GidonFrischkorn added this to the 1.0.0 milestone Feb 21, 2024
@GidonFrischkorn GidonFrischkorn linked an issue Feb 21, 2024 that may be closed by this pull request
@GidonFrischkorn GidonFrischkorn marked this pull request as draft February 21, 2024 15:02
@GidonFrischkorn
Copy link
Collaborator Author

GidonFrischkorn commented Feb 21, 2024

I just wanted to share my progress so far. The old/new SDT models should be implemented. For now, I added all noise distributions that can be easily accommodated (Normal, EVD, Cauchy, and Logistic). However, we might want to discuss if there that matches the theoretical discussions around SDT models.

The main things that still need to be done with respect to the old/new SDT models are:

  • add adequate tests for the SDT models
  • write a vignette explaining how to specify and use the SDT models.
  • add an example data set to try out the SDT models

@venpopov
Copy link
Owner

ok, I'll try to review what you have so far before we meet next week

@GidonFrischkorn
Copy link
Collaborator Author

GidonFrischkorn commented Feb 21, 2024

ok, I'll try to review what you have so far before we meet next week

There is no hurry. I just thought you might be interested. As the old/new models are mostly done. If you other things to do, I can also show you what I have done next week when we meet.

@venpopov
Copy link
Owner

yes, I just want to consider more broadly how the various different SDT models such as old/new,2-AFC, m-AFC, confidence ratings might fit together and what we would need to have as separate models

@GidonFrischkorn
Copy link
Collaborator Author

I found this article that should provide a good reference how to generalize the old/new SDT models to m-AFC SDT models: https://www.sciencedirect.com/science/article/pii/S0022249612000260

@venpopov
Copy link
Owner

venpopov commented Feb 22, 2024

I found this article that should provide a good reference how to generalize the old/new SDT models to m-AFC SDT models: https://www.sciencedirect.com/science/article/pii/S0022249612000260

good find! Given his interest and extensive reasearch and writing on SDT, SDT via regression and bayesian implementations, measurement in general (https://www.tc.columbia.edu/faculty/ld208/), maybe at some point we can contact him and chat about the package and get his thoughts on how the SDT is implemented, maybe get him involved if he wants to?

He's also written a bunch of programs for other software for SDT models, so I bet his experience and insight will be useful:

image

@GidonFrischkorn
Copy link
Collaborator Author

I thought the same. For the moment, I think we can implement plenty of things ourselves, but it would still be interesting to discuss with him about optimizations with respect to the SDT implementations.

He has also done some interesting work linking SDT to psychometric models such as IRT that I would find interesting to explore. But then again, there is already plenty of work on my desk. So maybe this is rather something for the future...

R/bmm_model_SDT.R Outdated Show resolved Hide resolved
R/bmm_model_SDT.R Outdated Show resolved Hide resolved
R/bmm_model_SDT.R Outdated Show resolved Hide resolved
R/bmm_model_SDT.R Outdated Show resolved Hide resolved
#' @param crit A vector of numerical values for the decision criterion.
#' @param stimulus A character vector ("signal"/"noise", "old"/"new") or
#' integer vector (1/0) coding which stimuli were shown alongside the observed responses,
#' or for which to generate data for. If no information is provided for `rSDT` then we will
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the function only returns hits by default. I think this is probably for the best, and would rather remove the description than change the function. Or return a list of two vectors, one for hits and one for FAs

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some documentation to clarify this point. My thought was to make the distribution functions consistent with the implementation as bmmmodel.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can have a look if this resolves your concerns.


#' @rdname SDTdist
#' @export
rSDT <- function(n, size, dprime = 1, crit = dprime/2, stimulus = 1, dist_noise = "normal") {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be good to have an option to return proportions instead of counts

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... do you mean that we return the proportion of hits/FA instead of the count. Why would users need these? If they want they can directly compute them from returnedCounts/size, but they would loose the information on how many trials the proportion was calculated on.

Maybe I am misunderstanding something, but my thought was to make these functions consistent with the model implementation.

R/distributions.R Outdated Show resolved Hide resolved
nTrials <- model$resp_vars$nTrials
brms_formula <- brms::bf(paste0(response," | ", "trials(",nTrials,")", " ~ dprime*",stimulus," - crit" ), nl = TRUE)
} else {
brms_formula <- brms::bf(paste0(response," ~ dprime*",stimulus," - crit"),nl = TRUE)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this works for symmetric distributions, but for assymetric ones it gives the wrong part of the cdf. Will explain in person

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I have checked with the parameter recoveries I ran, this implementation does recover the parameters well using the cloglog link for Gumbel noise. But we should test this thoroughly to make sure that I did not miss anything.

venpopov and others added 2 commits February 28, 2024 12:12
- make "normal" default noise distribution
- remove unnecessary functions
- Improve documentation
- move `stimulus` and `nTrials` to other_vars
@GidonFrischkorn
Copy link
Collaborator Author

Things that need to be added for finishing up the Old/New Recognition SDT models.

  • vignette introducing the model and providing an example how to fit the models
  • unit tests for the added functions
  • add an example data set to fit the models
  • evaluate parameter recovery more systematically

@GidonFrischkorn
Copy link
Collaborator Author

I will see that I take care of most of the to dos in the next week.

@venpopov
Copy link
Owner

venpopov commented Mar 1, 2024

Perfect, and I can finish the review and test it with the code you sent in the meantime

@venpopov venpopov removed this from the 1.0.0 milestone May 22, 2024
- update the already implemented SDT models to the current bmm version
- add an info `custom_bmf2bf` to all model objects to handle the `bmmformula` to `brmsformula` translation more efficiently.
@GidonFrischkorn GidonFrischkorn added this to the 1.1.0 milestone Jun 14, 2024
@GidonFrischkorn
Copy link
Collaborator Author

I just found this package implementing EV and UEV SDT models in STAN with a newly proposed link function that allows to fit hierarchical models also for the thresholds: https://github.com/boryspaulewicz/bhsdtr2/tree/master

Maybe we should have a look at how they coded this in STAN and see if we can adapt this for our package. Generally, the principle for predicting the model parameters looks pretty similar, with the difference that they do interact with brms at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR - minor Pull-request should update minor version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement SDT models
2 participants