giabaio / BCEA

Bayesian Cost Effectiveness Analysis. Given the results of a Bayesian model (possibly based on MCMC) in the form of simulations from the posterior distributions of suitable variables of costs and clinical benefits for two or more interventions, produces a health economic evaluation. Compares one of the interventions (the "reference") to the others ("comparators"). Produces many summary and plots to analyse the results
https://gianluca.statistica.it/software/bcea/
GNU General Public License v3.0
40 stars 16 forks source link

what to do with `evppi()` when `eff` & `cost` have different lengths to input parameters `inputs`? #53

Closed n8thangreen closed 9 months ago

n8thangreen commented 10 months ago

While writing the (regression) unit tests to replace the current internal evppi() function with the {voi} package version, an error occured. For the smoking example, the data I have has different number of rows for the cost and effectiveness matrices and the parameter posterior sample size. They should correspond. BCEA::evppi() actually doesn't throw an error so I don't know how it handles this or if its done correctly.

This is the voi::evppi() error:

Error in check_outputs_matrix(outputs$c, inputs, "outputs$c") : 
  Number of rows of `outputs$c` (500) should equal the number of rows of `inputs` (1000)

This is the test code:

  data(Smoking, package = "BCEA")
  treats <-
    c("No intervention", "Self-help",
      "Individual counselling", "Group counselling")

  bcea_smoke <- bcea(eff, cost, ref = 4, interventions = treats, Kmax = 500)
  inp <- createInputs(smoking_output)
  EVPPI_smoke <- BCEA::evppi(bcea_smoke, param_idx = c(2,3), inp$mat, h.value = 5e-7)

  # error
  voiEVPPI <- voi::evppi(bcea_smoke[c("e","c","k")], pars = c("d.3.", "d.4."), inputs = inp$mat, h.value = 5e-7)
n8thangreen commented 10 months ago

@giabaio any ideas?..

giabaio commented 10 months ago

I struggle to replicate this... Are you on the 2.4.4 version of BCEA? Is the object smoking_output available by default?

n8thangreen commented 10 months ago

Hi, You're right that I had taken out the jags output when sending to CRAN previously. I think I thought it was an issue but probably wasnt the actual problem.

I'd put it back in to the project recently to do the evppi testing, see: https://github.com/n8thangreen/BCEA/commit/38ff5ff828ee29db9920439b999163e7c7e1217f

I think this is the code taken wholesale from the BCEA book.

Smoking.Rmd is in the refactor-remove-evppi branch or I can send it to you separately?

giabaio commented 9 months ago

One more on this, before we can perhaps close this: I've modified the smoking_output object under data/Smoking.RData to select only the relevant matrix of simulations. This actually makes sense, because to use this object for the createInputs() and then evppi() example, we don't need it to be an rjags object --- it can be (as it can be a bugs or stan object, but a simple matrix will just do...). With this change (and by updating the GitHub repo for plotrix --- BTW: I think it's not orphaned any more, so perhaps we don't even need to add that repo in the Remotes field of our DESCRIPTION file) it now passes all the checks, except for AppVeyor. @n8thangreen given that we have included the check under Windows with the normal flow, do we need to keep AppVeyor? Or could we just remove that too?...

n8thangreen commented 9 months ago

Thanks for fixing this. After syncing the my fork its passing all the CHECKS in the GH Action. I'll remove the AppVeyor thing; I think thats just a legacy thing anyway

giabaio commented 9 months ago

[like] Baio, Gianluca reacted to your message:


From: Dr Nathan Green @.> Sent: Tuesday, November 21, 2023 10:24:12 AM To: giabaio/BCEA @.> Cc: Baio, Gianluca @.>; Mention @.> Subject: Re: [giabaio/BCEA] what to do with evppi() when eff & cost have different lengths to input parameters inputs? (Issue #53)

⚠ Caution: External sender

Thanks for fixing this. After syncing the my fork its passing all the CHECKS in the GH Action. I'll remove the AppVeyor thing; I think thats just a legacy thing anyway

— Reply to this email directly, view it on GitHubhttps://github.com/giabaio/BCEA/issues/53#issuecomment-1820633828, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACCRVSODCIGKX5GW2ZJXRXDYFR6MZAVCNFSM6AAAAAA62SQZC2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRQGYZTGOBSHA. You are receiving this because you were mentioned.Message ID: @.***>