mathesong / kinfitr

kinfitr: PET Kinetic Modelling Using R
Other
33 stars 7 forks source link

twotcm with purrr:pmap does not work after updating to 0.3.0 version - throws error #2

Closed pontusps closed 6 years ago

pontusps commented 6 years ago

After the upgrading to v0.3.0 i get an weird error using kinfitr::twotcm and purrr:pmap, se below:

#Make function for fitting 2TCM
fit2tcm <- function(tacs, input, delayFit) {
  twotcm(t_tac = tacs$time, tac = tacs$TAC, input = input, weights = tacs$Weights,
         inpshift = delayFit$par$inpshift, vB=delayFit$par$vB) 
}

#Fit 2TCM
TidyDataNestedLong %>%  
  mutate(fit_2tcm = pmap(list(TACs, input, delayFit), fit2tcm))

Throws the following error:

Error in mutate_impl(.data, dots) : Evaluation error: argument 6 matches multiple formal arguments.

When I downgrade to kinfitr 0.2.0 everything works beautifully, so I'm pretty sure that its something funky with 0.3.0. I cant find anything in the documentation suggesting that the code above is missing a newly added mandatory inarg. Not sure but looks like the only thing that's new is the nls.multstart inarg. Did something maybe go funky-wonky when this was added to kinfitr::twotcm?

For now, my quick-fix is to just run kinfitr::twotcm on 0.2.0, then upgrade to 0.3.0 and then run everything else. 😃

Edit: Downgrading gets around the error above for now: devtools::install_github("mathesong/kinfitr@v0.2.0") but means that nls.multstart can't be used.

mathesong commented 6 years ago

Your error message points you to the 6th input argument, vB. When you read the documentation, you will see that there is no input argument called vB any longer. Since 0.3.0, this input argument is called vB_fixed.

Thanks for raising the issue though! This is a breaking change, but was necessary to tidy up some other functions.

mathesong commented 6 years ago

Ok, so you actually got me started on fixing something that I've been meaning to fix for a long time, which was the reason I had to change all the vB's to vB_fixed. It didn't take quite as long as I'd anticipated, and I'm happy to report that it's now back to vB, and the version is 0.3.1. And there were some issues with running nls.multstart on 1tcm and 2tcm that I'd not spotted before which this brought to the surface. So this was actually super valuable in the end! Thanks :).

mathesong commented 6 years ago

Gimme a little bit before you start working with the package again. Realised I was a silly billy when implementing all that and made a bit of a booboo.

mathesong commented 6 years ago

Ok, things are go again. Though it does fail its new CI builds, which is annoying, but I'm going to have to figure out what that's all about another time.