config-i1 / greybox

Regression model building and forecasting in R
30 stars 7 forks source link

Noticed potential namespace conflict... #54

Closed Steviey closed 3 years ago

Steviey commented 3 years ago

Don't know if it's a bug but... greybox seems to occupy the global namespace of function forecast() if not explicitly set. This means, if we want to use both in the same script (Hyndman's and Ivan's), we have to address function forecast() in the right namespace...

Hyndman-example:

arima.forecast <- forecast::forecast(arima.model,h=myH,xreg=newRegressors,biasadj=T) 
vs.:
arima.forecast <- forecast(arima.model,h=myH,xreg=newRegressors,biasadj=T) 

global::bAlgos(i, f, "prediction") ~/R/daScript.R:59721:12
 2. │ └─global::my_forecasts(...) ~/R/daScript.R:59458:12
 3. │   ├─greybox::forecast(...) ~/R/daScript.R:48444:12
 4. │   └─greybox:::forecast.default(...)
 5. │     ├─stats::predict(object, ...)
 6. │     └─stats:::predict.Arima(object, ...)
 7. │       └─base::stop("'xreg' and 'newxreg' have different numbers of columns")

..._Another candidate: fabletools::forecast(newdata=future)

config-i1 commented 3 years ago

Hi John,

So, does it work or not? I tried to fix this issue in greybox v0.7.0, and my tests showed that if you load both packages, forecast(model) works for both greybox and forecast functions no matter the order of loaded packages. Is it not the case?

Steviey commented 3 years ago

Ubuntu 16 LTS, R 4.x, greybox v0.7.0

Hi Ivan, it did not work until I set the correct namespace. Unfortunately I have a 50k+ line script, where I forgot to set namespaces explicitly. After an update of all packages, nothing works as expected anymore. It seems now I have the same problems with function predict() ;-). As beginner, I did not realize, that there are so many package developers out there, using the same function names.

config-i1 commented 3 years ago

Hi John,

I tried to address this issue in 0.7.0, and my tests showed that it worked. Apparently, I was wrong. A couple of questions that might help me resolve it:

  1. Do you have fable package installed?
  2. What is the order of loading packages in your case?

I'm not sure why predict() would stop working, as I haven't done anything with that function. Also, for the time being try loading greybox first and then forecast, and see if it works.

Steviey commented 3 years ago

The order is forecast > fable > greybox. Since I load a lot of packages initially, instead of in functions, which should be, by the way- a 'wrong practice', told me Prof. Hyndman via E-Mail, I would guess, it would be best practice (for me) to address namespaces explicitly. The problem with function predict() does not come with your packages Ivan.

config-i1 commented 3 years ago

Well, yes using "::" is a safe option, but I've tried to resolve the issues you mentioned in the previous version. Does this order work?

library(greybox) library(forecast) library(fable)

I'll see if I can fix this anyway.

Steviey commented 3 years ago

Ubuntu v.: 1.16 LTS, R v.: 4.x, greybox v.: 0.7.0

Hello Ivan,

I can't replicate the exact error, but I get the following: Case 1: order: forecast > fable > greybox Result: no error, but warning: Warning message: In .recacheSubclasses(def@className, def, env) : undefined subclass "numericVector" of class "Mnumeric"; definition not updated Case 2: order: greybox > forecast > fable Result: no error, no warning

Cases triggered as separated RStudio-session.

hope this helps, cu

config-i1 commented 3 years ago

It does, thank you! I'll fix this in v1.0.0 in a week or so.

Steviey commented 3 years ago

Ubuntu v.: 1.16 LTS, R v.: 4.x, greybox v.: 0.7.0

Update: Sorry, I had double entries in the list of includes, retesting...

Case 1: order: forecast > fable > greybox Result: no error, but warning: Warning message: In .recacheSubclasses(def@className, def, env) : undefined subclass "numericVector" of class "Mnumeric"; definition not updated

Case 2: order: greybox > forecast > fable Result: no error, but warning: Warning message: In .recacheSubclasses(def@className, def, env) : undefined subclass "numericVector" of class "Mnumeric"; definition not updated

Cases triggered as separated RStudio-sessions.

Original includes (Case 2): ` packList<-list(

"devtools"

    "tigerstats"
    ,"Metrics"
    ,"nlme"
    ,"plyr"
    ,"dplyr"
    ,"rpart"
    ,"randomForest"
    ,"party"
    ,"Cubist"
    #,"rJava" nur ueber apt get
    ,"RWeka"#vorher rjava-hint und rjava mit apt -get installieren
    ,"extraTrees"# Cannot allocate memory
    ,"ipred"
    ,"kernlab"
    ,"nnet"
    ,"pls"
    ,"glmnet"
    ,"earth"
    ,"gbm"
    #,"forecast"
    ,"forecastHybrid"
    ,"data.table"
    ,"evtree"
    ,"timetk"
    ,"tidyquant"
    ,"digest"
    ,"recipes"
    ,"Hmisc"
    ,"MASS"
    ,"gam"
    ,"pracma"
    ,"inTrees"
    #,"h2o"
    ,"RSQLite"
    #,"forecast"
    ,"tseries"
    ,"mlbench"
    ,"neuralnet"# Cannot allocate memory (install mit frischer session)
    ,"nnls"# Cannot allocate memory (install mit frischer session)
    ,"spls"
    ,"bridgedist"# für bridge sonst not available for R 3.5.1
    ,"rpart"# reihenfolge vor mboost
    ,"mboost"
    ,"spikeslab"
    ,"e1071"#
    #,"svmLinear3Search" kommt vermutlich über caret rein not available for R 3.5.1
    #,"krlsRadialSearch"  not available for R 3.5.1
    ,"nnfor"#fuer elm
    #,"elmSearch" not available for R 3.5.1
    #,"mlp_R" not available for R 3.5.1
    #,"BstLm" not available for R 3.5.1
    ,"kknn"
    ,"partDSA"
    #,"treebag" , kommt durch caret rein, not available for R 3.5.1
    ,"nodeHarvest"
    ,"penalized"
    ,"msaenet"
    ,"brnn"
    ,"qrnn"
    #,"bayesglm" #not available for R 3.5.1
    ,"monmlp"
    ,"bartMachine" # rJava kein install gepackt
    #,"elmNN" # not available for R 3.5.1
    ,"GAMBoost" # tested, FAILED!!!
    ,"KRLS"
    ,"RSNNS" # mlp
    ,"bst"
    ,"plyr"
    ,"lme4" # reihenfolge vor arm
    ,"arm"
    ,"doParallel" # h2o macht es intern parallel
    ,"foreach"# h2o macht es intern parallel
    ,"inTrees"
    ,"ipred"
    ,"mlr"
    ,"caret"# manuell per rstudio, Reihenfolge caret als letztes, weil es von oben libs braucht.
    #,"caretEnsemble"# install manuell per rstudio
    #,'catboost'
    ,"prophet" #
    ,"deepnet"
    ,"leaps"
    ,"monomvn"
    ,"elasticnet"
    ,"rqPen"
    ,"fastICA"
    ,"Rborist"
    ,"quantregForest"
    ,"import"
    ,"LiblineaR"
    ,"rlang"
    ,"gtable"
    ,"ggplot2"
    ,"ggrepel"
    #,"TSstudio"
    ,"CombMSC"
    ,"dbplyr"
    #,"doSNOW"
    #,"rBayesianOptimization"
    ,"stringr"
    #sampleData
,"lobstr"
#,"fable"
,"tsibble"
,"lubridate"
,"feasts"
,"tsibbledata"        
,"tidyverse"    
,"tidymodels"
,"modeltime"
,"timetk"
,"lubridate"
,"rsample"       
,"vip"
#,"distributional"
,"smooth"
#,"greybox"
#,"here"

,"greybox"
,"forecast"
,"fable"
)`
config-i1 commented 3 years ago

It should work with the recent version of greybox now. It might give warnings, but forecast() works with objects from greybox / forecast well. I could not test fable though, so this one is not guaranteed. I'd appreciate any feedback on that package.

Steviey commented 3 years ago

I didn't got the warning for a while using greybox latest- I believe. Today I updated to latest smooth and get it again. I'm not really sure here, because I had no time to sort it out (another big project in the same code).

In addition: Warning message:
In .recacheSubclasses(def@className, def, env) :
  undefined subclass "numericVector" of class "Mnumeric"; definition not updated
config-i1 commented 3 years ago

Seems like an issue of Matrix package.

mmaechler commented 3 years ago

No, this is not issue of the Matrix package. It is an issue of using packages which import Matrix and which --- in this very special case --- need to be re-installed.

This issue came up several times and the solution in the end was always the same: namespace-loading of a package that in some way depends on Matrix but was installed with an "old" version of Matrix.

config-i1 commented 3 years ago

Okay. I will reopen this as an issue then, but I would need a reproducible example to track this and fix, if this is indeed a greybox-related issue. The only thing I can say for now is that greybox does not rely on any specific version of Matrix - it just imports sparse.model.matrix from it, and that's it.

Steviey commented 3 years ago

Hey Ivan,

if you make a dev-version with the new matrix-version, I can test it for you on my corresponding system. It still seems to be possible, to be OS-dependent for me.

In regard to a reproducible example I can say, I'm using smooth and greybox the same way as we discussed (with examples). Nothing changed since then on my side.

Or do I have to reinstall matrix locally? I'm confused :-)

cu steive

config-i1 commented 3 years ago

Steive,

What happens if you reinstall Matrix locally? Does it fix the issue?

mmaechler commented 3 years ago

Sorry, yes, you all seem to remain confused: I wrote

It is an issue of using packages which import Matrix and which --- in this very special case --- need to be re-installed.

It's all the other Matrix-importing packages which need to be re-installed, not Matrix itself.

config-i1 commented 3 years ago

I removed the dependence on Matrix package for now. I only used one function from it (sparse.model.matrix()), and it wasn't very necessary anyway. So, this issue should be resolved now in the greybox v1.0.1.41003... at least for now, until I need the Matrix package again :)