AlbertoAlmuinha / boostime

The Tidymodels Extension for Time Series Boosting Models
Other
55 stars 6 forks source link

What is original in your package that isn't a direct replica of the ModelTime package? #1

Closed AdrianAntico closed 3 years ago

AdrianAntico commented 3 years ago

@AlbertoAlmuinha @business-science

Hi Alberto,

I looked into the Business Science Repo 'ModelTime' by Matt Dancho and investigated his code for arima_boost() and then compared it to your code for boost_arima(). Your code is basically an exact replica of his. I thought to check because I saw that you forked his package and then later released boosttime. Is there a reason you decided to create your own package that is that similar to his? What is different about it or how do you plan on making it different? To me, this seems rather unethical, even if Matt Dancho gave you the thumbs up to copy his stuff. Why not simply contribute to ModelTime?

https://github.com/AlbertoAlmuinha/boostime/blob/master/R/parsnip-arima_boost.R https://github.com/business-science/modeltime/blob/master/R/parsnip-arima_boost.R

@business-science

arima_boost <- function(mode = "regression", seasonal_period = NULL,
                        non_seasonal_ar = NULL, non_seasonal_differences = NULL, non_seasonal_ma = NULL,
                        seasonal_ar = NULL, seasonal_differences = NULL, seasonal_ma = NULL,
                        mtry = NULL, trees = NULL, min_n = NULL,
                        tree_depth = NULL, learn_rate = NULL,
                        loss_reduction = NULL,
                        sample_size = NULL, stop_iter = NULL
) {

  args <- list(

    # ARIMA
    seasonal_period           = rlang::enquo(seasonal_period),
    non_seasonal_ar           = rlang::enquo(non_seasonal_ar),
    non_seasonal_differences  = rlang::enquo(non_seasonal_differences),
    non_seasonal_ma           = rlang::enquo(non_seasonal_ma),
    seasonal_ar               = rlang::enquo(seasonal_ar),
    seasonal_differences      = rlang::enquo(seasonal_differences),
    seasonal_ma               = rlang::enquo(seasonal_ma),

    # XGBoost
    mtry                      = rlang::enquo(mtry),
    trees                     = rlang::enquo(trees),
    min_n                     = rlang::enquo(min_n),
    tree_depth                = rlang::enquo(tree_depth),
    learn_rate                = rlang::enquo(learn_rate),
    loss_reduction            = rlang::enquo(loss_reduction),
    sample_size               = rlang::enquo(sample_size),
    stop_iter                 = rlang::enquo(stop_iter)
  )

  parsnip::new_model_spec(
    "arima_boost",
    args     = args,
    eng_args = NULL,
    mode     = mode,
    method   = NULL,
    engine   = NULL
  )

}

And then your code. It's nearly a carbon copy.

@AlbertoAlmuinha

boost_arima <- function(mode = "regression", seasonal_period = NULL,
                        non_seasonal_ar = NULL, non_seasonal_differences = NULL, non_seasonal_ma = NULL,
                        seasonal_ar = NULL, seasonal_differences = NULL, seasonal_ma = NULL,
                        tree_depth = NULL, learn_rate = NULL, mtry = NULL, trees = NULL, min_n = NULL,
                        sample_size = NULL, loss_reduction = NULL) {

  args <- list(

    # ARIMA
    seasonal_period           = rlang::enquo(seasonal_period),
    non_seasonal_ar           = rlang::enquo(non_seasonal_ar),
    non_seasonal_differences  = rlang::enquo(non_seasonal_differences),
    non_seasonal_ma           = rlang::enquo(non_seasonal_ma),
    seasonal_ar               = rlang::enquo(seasonal_ar),
    seasonal_differences      = rlang::enquo(seasonal_differences),
    seasonal_ma               = rlang::enquo(seasonal_ma),

    # Catboost/LightGBM
    tree_depth                = rlang::enquo(tree_depth),
    learn_rate                = rlang::enquo(learn_rate),
    mtry                      = rlang::enquo(mtry),
    trees                     = rlang::enquo(trees),
    min_n                     = rlang::enquo(min_n),
    sample_size               = rlang::enquo(sample_size),
    loss_reduction            = rlang::enquo(loss_reduction)
  )

  parsnip::new_model_spec(
    "boost_arima",
    args     = args,
    eng_args = NULL,
    mode     = mode,
    method   = NULL,
    engine   = NULL
  )

}
AlbertoAlmuinha commented 3 years ago

Hello,

First of all, if you understood what is being talked about you would realize that a contribution to Modeltime is not possible because Catboost simply cannot be sent to CRAN, hence that option has been discarded. If you had looked more closely you would have seen that I am also a Modeltime contributor.

Secondly, about the codes you mention being copies, of course they are the same. They are the model definitions in tidymodels and one is using an Arima with XGBoost while here you are using Arima with Catboost and LightGBM, so the whole Arima and Prophet part is the same, and it is as it should be besides. What doesn't make sense is that there are 50 different definitions of the same model depending on the mood of the developer.

Greetings,

Steviey commented 2 years ago

@AdrianAntico You did not understood the concept ;-)

AdrianAntico commented 2 years ago

@Steviey Build something useful and then talk. As it stands, you only have forks of other people's works in your repo which leads me to believe you have no clue how to do anything. But go ahead and keep cheerleading these useless projects