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

ranger, bootstrap for time series #482

Closed
wants to merge 21 commits into from

Conversation

hyanworkspace
Copy link

joint work with Benjamin Goehry

@mnwright
Copy link
Member

mnwright commented Feb 4, 2020

Thanks for the PR! Do you have any examples or tests?

Also, I wonder what is the best approach to include this. I see 3 options (feel free to propose another):

  1. Merge into ranger as it is
  2. Create an additional R function (e.g. rangerts) in the package
  3. Fork the repo and have an additional package for time series

The problem with 1. is that the number of arguments in ranger() grows and the interface as well as the code might be confusing. It might also be harder to add additional time series related stuff. With option 2. we keep the interface cleaner but not the C++ code (maybe that is also confusing). With option 3. we would have an additional repo to maintain but it would also be cleaner to separate the time series code.

What are your thoughts on this?

@hyanworkspace
Copy link
Author

Thanks for your message! We have discussed internally, and we think option 1 might be the best. In fact, our modifications focus only on the bootstrap step when building trees, and computing variable importance by permutation. The arguments we added can be hidden quite well, and most of time, they are inactive.
Maybe I can push some testthat module for time series part, will do it very soon!

@Shafi2016
Copy link

Hello, Is bootstrap for time series implemented with a ranger now? Is there any example?

@hyanworkspace
Copy link
Author

Hi the package rangerts is available here: https://github.com/hyanworkspace/rangerts.
We are waiting (if still possible) to be merged into the ranger package but you can test it out.

@Shafi2016
Copy link

Shafi2016 commented Dec 7, 2021

Thank you so much.

@Shafi2016
Copy link

Hello @hyanworkspace, Have you thought of adding rangerts to modeltime (https://business-science.github.io/modeltime/articles/extending-modeltime.html)

@athammad
Copy link

Hi guys,

I am jumping on this since I am very interested in rangerts. I would like to know if there is any chance to use the options of rangerts directly within caret. I have tried to run the following example but I don't think there is any impact on the results.

rangerTS<-train(y=Y,x=X,
                   trControl = train_control,
                   tuneLength = 20,
                   method = 'ranger',
                   quantreg=TRUE, bootstrap.ts = "moving", block.size = 7)

@mnwright
Copy link
Member

Sorry for the long silence. In addition to the lack of time to deal with this, I'm still not happy with adding four additional parameters to ranger() (which might also be confusing to users not using time series).

We could also leave it as it is now (separate repo). Actually, you have quite some information on your repo, and now you get the credit for the package.

Btw., we should create a lists if forks/extensions that build on ranger and link to them here in the repo.

@mnwright
Copy link
Member

I think we leave it with the separate package.

See #724 for linking to repo.

@mnwright mnwright closed this May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants