refactor `createInput()`

[n8thangreen]

• is createInput() used anywhere else other than in evppi()? -> can we move it inside of evppi()

• why does createInputs() return a list with the parameter colnames? Where is this then used? -> can we remove this? or at least move it to an attribute? https://github.com/n8thangreen/BCEA/blob/d2c2c63115c13e49683ac70a6441f84e5bcf46bb/R/CreateInputs.R#L61

• what does cbind() do here? https://github.com/n8thangreen/BCEA/blob/d2c2c63115c13e49683ac70a6441f84e5bcf46bb/R/CreateInputs.R#L43

[giabaio]

• Yes. createInputs is specifically used in evppi (and I can't think of another place where it would helpful). So in a sense we could move it as a hidden function inside the new version of evppi...

• I think that's when you allow the use of variable names in the call to evppi.

• Can't remember why we had it this way, but could it be because it then generates a matrix, which is what evppi wants? In any case, we could "tidyverse" evppi and use better ways of restructuring the matrix of parameters simulations...

[n8thangreen]

Thanks, We could keep the argument but if its not supplied then it does createInputs internally. I don't think the user really needs to see this happening.

• createInputs() returns a dataframe with column names anyway so its sort of giving the same information twice. It would also mean you don't have to do inp$mat when passing it to evppi(), which I think is a good idea because I don't see why the user should have to remember to do that really.

• I'll check if it changes to a matrix. Maybe it would be clearer to as something like as.matrix?

[annaheath]

The reason you would want createInputs() separately is because you will often want to calculate EVPPI for different parameter sets but they would come from the same matrix from the createInputs() function. As createInputs() is relatively slow, I don't think you want to use it every time you run the evppi() function. For the parameters, you probably can delete and just return an inputs matrix. I've no idea why the cbind is there - I have a vague memory of having difficulties with that line of code so potentially that might be the reason it is there but from the written code I think it can be deleted.

[annaheath]

I can see another problem with having createInputs() inside the evppi() function because the createInputs() function deletes redundant columns from matrix - if you chose numbered columns to calculate EVPPI then the numbers would change when you updated the matrix. You could also delete a column that the user was trying to compute EVPPI for.

[n8thangreen]

I see. Some ideas could be:

• evppi() could have an update() type of function for when you rerun it so that you don't have to redo createInputs()?
• I could try and speed up createInputs()
• maybe we could name the columns with the character of the column number?
• these sort of issues you mention are perfect for encapsulating in tests! If you have some examples of when having createInputs() inside evppi() would not work I can make a test and then when I make the changes I'll know I haven't broken it.

[annaheath]

What is the advantage of using evppi() and update() as opposed to createInputs() and evppi()? Can you associate columns with 2 names? I'm just thinking that you may also want to call the column by its parameter name as well?

[giabaio]

Hi both, Thanks for this --- that's very helpful. I think we do want to allow the possibility of selecting a vector of numbers of columns as well as named vectors of names... Sounds like probably we can make createInputs() a bit quicker, but probably keep it separated from evppi... G

On Mon, 26 Apr 2021 at 17:47, Anna Heath @.***> wrote:

What is the advantage of using evppi() and update() as opposed to createInputs() and evppi()? Can you associate columns with 2 names? I'm just thinking that you may also want to call the column by its parameter name as well?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/n8thangreen/BCEA/issues/41#issuecomment-826990108, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCRVSIVBSUZ5IXRHUNBTPDTKWKIDANCNFSM43OGQDLQ .

--

Gianluca Baio

Professor of Statistics & Health Economics Department of Statistical Science University College London 1-19 Torrington Place, London WC1E 6BT, UK Telephone +44(0)20 7679 1248 (internal: 41248) Twitter: @stats_UCL https://twitter.com/stats_UCL Website: http://www.homepages.ucl.ac.uk/~ucakgba http://www.homepages.ucl.ac.uk/~ucakgba/ Blog: http://www.statistica.it/gianluca/blog/

[image: https://twitter.com/gianlubaio] https://twitter.com/gianlubaio https://www.linkedin.com/in/gianluca-baio-b893879/ https://github.com/giabaio https://scholar.google.com/citations?user=ro0QvGsAAAAJ&hl=en https://www.researchgate.net/profile/Gianluca_Baio[image: https://www.linkedin.com/in/gianluca-baio-b893879/] https://www.linkedin.com/in/gianluca-baio-b893879/ This email may reach you outside of your working hours. If that's the case, please do not feel pressure to reply immediately.

[n8thangreen]

I'm thinking of it in a kind of encapsulation way. We don't want to expose the workings of the algorithm if we don't have to. The user interface is just 'do an evppi' which could include 'update my evppi'. So you're working on the same type of object. I think asking them to first create inputs should really been handled privately because they don't need to know about the intermediate step. I know it sounds pedantic but I think it would be more intuitive.

[giabaio]

I don't know if I'm misunderstanding this but do you mean having a evppi command and an update.evppi command? Or is it about the philosophy of the function?

On Mon, 26 Apr 2021 at 19:25, Dr Nathan Green @.***> wrote:

I'm thinking of it in a kind of encapsulation way. We don't want to expose the workings of the algorithm if we don't have to. The user interface is just 'do an evppi' which could include 'update my evppi'. So you're working on the same type of object. I think asking them to first create inputs should really been handled privately because they don't need to know about the intermediate step. I know it sounds pedantic but I think it would be more intuitive.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/n8thangreen/BCEA/issues/41#issuecomment-827052559, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCRVSO4KOYFRHMHA46DQ3DTKWVZXANCNFSM43OGQDLQ .

--

Gianluca Baio

Professor of Statistics & Health Economics Department of Statistical Science University College London 1-19 Torrington Place, London WC1E 6BT, UK Telephone +44(0)20 7679 1248 (internal: 41248) Twitter: @stats_UCL https://twitter.com/stats_UCL Website: http://www.homepages.ucl.ac.uk/~ucakgba http://www.homepages.ucl.ac.uk/~ucakgba/ Blog: http://www.statistica.it/gianluca/blog/

[image: https://twitter.com/gianlubaio] https://twitter.com/gianlubaio https://www.linkedin.com/in/gianluca-baio-b893879/ https://github.com/giabaio https://scholar.google.com/citations?user=ro0QvGsAAAAJ&hl=en https://www.researchgate.net/profile/Gianluca_Baio[image: https://www.linkedin.com/in/gianluca-baio-b893879/] https://www.linkedin.com/in/gianluca-baio-b893879/ This email may reach you outside of your working hours. If that's the case, please do not feel pressure to reply immediately.

[n8thangreen]

You could also delete a column that the user was trying to compute EVPPI for.

Could we force createInput() to keep specified columns then? Or just return an error message "parameter not available..."?

[n8thangreen]

I don't know if I'm misunderstanding this but do you mean having a evppi command and an update.evppi command? Or is it about the philosophy of the function?

I think both.

[giabaio]

I think the way I was thinking about it in the first place is that you do createInput only once to remove the columns that are linearly dependent on others; then you are good to go for any combination of parameters for which you want to compute the evppi (and that's only one type of object there, I think --- you have

inp = createInputs(MCMC_object) evppi(...)

and you can specify any vector of parameters.

Maybe there are more clever ways to check linear dependence in the matrix of simulations?

On Mon, 26 Apr 2021 at 21:17, Dr Nathan Green @.***> wrote:

I don't know if I'm misunderstanding this but do you mean having a evppi command and an update.evppi command? Or is it about the philosophy of the function?

I think both.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/n8thangreen/BCEA/issues/41#issuecomment-827120039, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCRVSJAKAKYSUD6L6PRIMDTKXC7BANCNFSM43OGQDLQ .

--

Gianluca Baio

Professor of Statistics & Health Economics Department of Statistical Science University College London 1-19 Torrington Place, London WC1E 6BT, UK Telephone +44(0)20 7679 1248 (internal: 41248) Twitter: @stats_UCL https://twitter.com/stats_UCL Website: http://www.homepages.ucl.ac.uk/~ucakgba http://www.homepages.ucl.ac.uk/~ucakgba/ Blog: http://www.statistica.it/gianluca/blog/

[image: https://twitter.com/gianlubaio] https://twitter.com/gianlubaio https://www.linkedin.com/in/gianluca-baio-b893879/ https://github.com/giabaio https://scholar.google.com/citations?user=ro0QvGsAAAAJ&hl=en https://www.researchgate.net/profile/Gianluca_Baio[image: https://www.linkedin.com/in/gianluca-baio-b893879/] https://www.linkedin.com/in/gianluca-baio-b893879/ This email may reach you outside of your working hours. If that's the case, please do not feel pressure to reply immediately.

[n8thangreen]

the first call to evppi() can return something that kind of inherits from bcea and captures the original call so when you call it again, e.g. with update.evppi(), it doesnt have to rerun createInputs() again internally.

[giabaio]

That may do...

On Tue, 27 Apr 2021 at 08:56, Dr Nathan Green @.***> wrote:

the first call to evppi() can return something that kind of inherits from bcea and captures the original call so when you call it again, e.g. with update.evppi(), it doesnt have to rerun createInputs() again internally.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/n8thangreen/BCEA/issues/41#issuecomment-827399553, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCRVSOBUSOXORQJVF3YYWTTKZU3DANCNFSM43OGQDLQ .

--

Gianluca Baio

Professor of Statistics & Health Economics Department of Statistical Science University College London 1-19 Torrington Place, London WC1E 6BT, UK Telephone +44(0)20 7679 1248 (internal: 41248) Twitter: @stats_UCL https://twitter.com/stats_UCL Website: http://www.homepages.ucl.ac.uk/~ucakgba http://www.homepages.ucl.ac.uk/~ucakgba/ Blog: http://www.statistica.it/gianluca/blog/

[image: https://twitter.com/gianlubaio] https://twitter.com/gianlubaio https://www.linkedin.com/in/gianluca-baio-b893879/ https://github.com/giabaio https://scholar.google.com/citations?user=ro0QvGsAAAAJ&hl=en https://www.researchgate.net/profile/Gianluca_Baio[image: https://www.linkedin.com/in/gianluca-baio-b893879/] https://www.linkedin.com/in/gianluca-baio-b893879/ This email may reach you outside of your working hours. If that's the case, please do not feel pressure to reply immediately.

[n8thangreen]

if we change then:

• need to include a deprecation warning message
• message that can't use parameters that are then dropped by createInputs()