mlverse / tft

R implementation of Temporal Fusion Transformers
https://mlverse.github.io/tft/
Other
26 stars 9 forks source link

Feedback on the API #20

Open dfalbel opened 2 years ago

dfalbel commented 2 years ago

I wrote a draft vignette on what I think would be a reasonable API for tft and would like to get some feedback on it. Ideally I'd love to have the same API as modeltime, but I am not sure how to fit it into its workflow because of a few critical differences:

  1. TFT handles differently each kind of predictors (static variables, known and unknown time varying variables) and data prep must be very different for each one.
  2. It can be used to forecast multivariate time series as well as multiple time series.
  3. It also produces multi-horizon forecasts - couldn't find how to do it with modeltime.
  4. Allow producing prediction intervals.

The write-up can be found here: https://mlverse.github.io/tft/articles/Getting-started.html

@cregouby, @mdancho84, @topepo would love know your opinion on it. Let me know if you have time for a quick look.

cregouby commented 2 years ago

👍🏽 for the horizon = 5 parameter, as current total_time_step is completely ugly. 👍🏽 for the tsibble in, tsibble out, as such

So it is fine for me globally. I'm still not comfortable with the "known" name for the last new_role, even if it comes from the paper, as no one can remember it without referring to a doc. But I can't find a better name... So this just adds a challenge for documentation clarity. And we have to manage the split of tsibble keys into former new_role="id" and new_role="static_input" that is not covered here. We could document first key will endorse the id role maybe...