richarddmorey / BayesFactor

BayesFactor R package for Bayesian data analysis with common statistical models.
https://richarddmorey.github.io/BayesFactor/
132 stars 49 forks source link

Non-zero callback return value doesn't always abort analyses #52

Closed jonathon-love closed 9 years ago

jonathon-love commented 9 years ago

the following code doesn't abort the analysis (i think it aborts part of the analysis):

library(BayesFactor)

y <- runif(200)
x <- rep(0:1, 100)
sub <- rep(1:100, each=2)

data <- data.frame(y, x, sub=as.factor(sub))

e <- new.env()
e$counter <- 0  # R by ref hack

callback <- function(...) {

    e$counter <- e$counter + 1

    if (e$counter == 1) {
        print("ABORTING ANALYSIS")
        return (as.integer(1))
    }

    as.integer(0)
}

generalTestBF(y ~ x * sub, data, callback=callback)
richarddmorey commented 9 years ago

Why do you think it aborts part of the analysis?

richarddmorey commented 9 years ago

Oh, I see what is happening. In response to your previous issue, I made it call the callback at the beginning and end, but it does not check for an abort. I'll fix this.

jonathon-love commented 9 years ago

your fix works, but if i change the line:

    if (e$counter == 1) {

to:

    if (e$counter == 2) {

then the analysis does not abort any more

whole code:

library(BayesFactor)

y <- runif(200)
x <- rep(0:1, 100)
sub <- rep(1:100, each=2)

data <- data.frame(y, x, sub=as.factor(sub))

e <- new.env()
e$counter <- 0  # R by ref hack

callback <- function(...) {

    e$counter <- e$counter + 1

    if (e$counter == 2) {
        print("ABORTING ANALYSIS")
        return (as.integer(1))
    }

    as.integer(0)
}

generalTestBF(y ~ x * sub, data, callback=callback)
richarddmorey commented 9 years ago

Yes, I had noticed that. It aborts the individual analysis, but then since the counter increments on every call, the next analysis is allowed to continue. What's the desired behaviour here? If the counter stays at 2, then all the subsequent analyses will be cancelled as well.

jonathon-love commented 9 years ago

the desired behaviour would be for a non-zero callback to abort the whole analysis.

it's true that in it's current state, if the callback continues to return non-zero, it will eventually cancel the whole analysis; but sometimes there can be ~1 second delay between callbacks. if there's 10 models, this is 10 seconds before the analysis is actually cancelled.

so aborting the whole analysis with one callback would be better for us.

richarddmorey commented 9 years ago

OK, I'll have to make sure that everything gets cleaned up, but I think this shouldn't be a problem.

jonathon-love commented 9 years ago

cheers

jonathon-love commented 9 years ago

while you're at it, how would you feel about callbacks returning a value of 1 through 999, rather than 1 through 1000 ?

the rationale being that a callback isn't necessary if the analysis is 100% complete.

richarddmorey commented 9 years ago

actually, I'm using 0 - 1000. But the numbers are arbitrary. If the callback gets called at all, then the analysis is, by definition, not complete, right?

On Tue, Feb 24, 2015 at 9:19 AM, Jonathon Love notifications@github.com wrote:

while you're at it, how would you feel about callbacks returning a value of zero through 999, rather than 1 through 1000 ?

the rationale being that a callback isn't necessary if the analysis is 100% complete.

— Reply to this email directly or view it on GitHub https://github.com/richarddmorey/BayesFactor/issues/52#issuecomment-75722975 .

Richard D. Morey Senior Lecturer School of Psychology, Cardiff University http://psych.cf.ac.uk/morey

jonathon-love commented 9 years ago

yup, but 1-999 simplifies the calculation of "percentage complete". it's easier to map 1 through 999 to .1% through 99.9%, than map 0 through 1000.

but it's not a big deal.