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

predict for bsignal seems broken #17

Closed
pfistfl opened this issue Aug 8, 2020 · 4 comments
Closed

predict for bsignal seems broken #17

pfistfl opened this issue Aug 8, 2020 · 4 comments

Comments

@pfistfl
Copy link

@pfistfl pfistfl commented Aug 8, 2020

The example from the vignette FLAM_fuel seems broken, the "predict" step does lead to errors

# From the vignette:
data(fuelSubset)
fuel <- fuelSubset
fuel$dUVVIS <- t(apply(fuel$UVVIS, 1, diff))
fuel$dNIR <- t(apply(fuel$NIR, 1, diff))
fuel$duvvis.lambda <- fuel$uvvis.lambda[-1]
fuel$dnir.lambda <- fuel$nir.lambda[-1]

modH2O <- FDboost(h2o ~ bsignal(UVVIS, uvvis.lambda, knots=40, df=4)
                    + bsignal(NIR, nir.lambda, knots=40, df=4)
                    + bsignal(dUVVIS, duvvis.lambda, knots=40, df=4)
                    + bsignal(dNIR, dnir.lambda, knots=40, df=4),
                    timeformula=~bols(1), data=fuel)

# Predict errors
predict(modH2O, fuel)
> Error in h(simpleError(msg, call)) :
>   error in evaluating the argument 'x' in selecting a method for function 'which': subscript out of bounds
> sessionInfo()
R version 4.0.2 (2020-06-22)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04 LTS

Matrix products: default
BLAS:   /usr/lib/x86_64-linux-gnu/openblas-pthread/libblas.so.3
LAPACK: /usr/lib/x86_64-linux-gnu/openblas-pthread/liblapack.so.3

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=de_DE.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=de_DE.UTF-8    LC_MESSAGES=en_US.UTF-8   
 [7] LC_PAPER=de_DE.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=de_DE.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] parallel  stats     graphics  grDevices utils     datasets  methods  
[8] base     

other attached packages:
[1] FDboost_0.3-3     mboost_2.9-2      stabs_0.6-3       checkmate_2.0.0  
[5] mlr_2.17.1.9004   testthat_2.3.2    ParamHelpers_1.14

@sbrockhaus
Copy link
Member

@sbrockhaus sbrockhaus commented Aug 19, 2020

The error occurs in mboost:::check_newdata(newdata, blg, mf) as the number of classes of the data in newdata and mf do no match in the following check:

idx <- which(!sapply(1:length(cl1), function(i) all(cl1[[i]] == 
      cl2[[i]])))

In newdata, the functional data has class "model.matrix", "array"
In mf, the functional data has class "AsIs".
Thus, the number of class arguments do not match and we get the error in which.

@sbrockhaus
Copy link
Member

@sbrockhaus sbrockhaus commented Aug 19, 2020

As a quick fix, you can set all the data matrixes of functional covariates to class "AsIs":

data(fuelSubset)
fuel <- fuelSubset
fuel$UVVIS <- I(fuel$UVVIS)
fuel$NIR <- I(fuel$NIR)
fuel$dNIR <- I(t(apply(fuel$NIR, 1, diff)))
fuel$dUVVIS <- I(t(apply(fuel$UVVIS, 1, diff)))
fuel$duvvis.lambda <- fuel$uvvis.lambda[-1]
fuel$dnir.lambda <- fuel$nir.lambda[-1]

sapply(fuel, class)
str(fuel)

modH2O <- FDboost(h2o ~ bsignal(UVVIS, uvvis.lambda, knots=40, df=4)
                  + bsignal(NIR, nir.lambda, knots=40, df=4)
                  + bsignal(dUVVIS, duvvis.lambda, knots=40, df=4)
                  + bsignal(dNIR, dnir.lambda, knots=40, df=4),
                  timeformula=~bols(1), data=fuel)

# Now, the predict works
predict(modH2O, fuel)
@sbrockhaus
Copy link
Member

@sbrockhaus sbrockhaus commented Aug 19, 2020

Setting all functional variable matrices to class "AsIS" works now and also used to work before. So we could change the example code accordingly and add a check in FDboost() for the class of functional variables telling the user that he/she has to set functional variables to class 'AsIs'.

Or we make sure that FDboost can also handle other classes for functional variables like a numeric matrix, as it did before.

What do you think @davidruegamer @Almond-S ?

@davidruegamer
Copy link
Member

@davidruegamer davidruegamer commented Aug 28, 2020

@sbrockhaus Thanks for looking into this and your workaround. I think predict should be consistent with the original fitting behaviour, e.g., if you can input numeric matrices into FDboost to get the initial fit, you should also be able to feed them into predict function without further pre-processing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.