Meta name bool #997
Meta name bool #997
Conversation
Model Benchmark
Model training plotsModel TrainingPeytonManningYosemiteTempsAirPassengers |
Codecov Report
@@ Coverage Diff @@
## main #997 +/- ##
==========================================
+ Coverage 89.89% 89.91% +0.01%
==========================================
Files 18 19 +1
Lines 4653 4649 -4
==========================================
- Hits 4183 4180 -3
+ Misses 470 469 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@alfonsogarciadecorral not sure if this needs to be added to a specific release, from my side this is more of a refactoring we can always merge into the master. @noxan what do you think? |
karl-richter
added
status: ready
neuralprophet/forecaster.py
Outdated
| @@ -705,6 +705,9 @@ def fit(self, df, freq="auto", validation_df=None, progress="bar", minimal=False | |||
| # When only one time series is input, self.id_list = ['__df__'] | |||
| self.num_trends_modelled = len(self.id_list) if self.config_trend.trend_global_local == "local" else 1 | |||
| self.num_seasonalities_modelled = len(self.id_list) if self.config_season.global_local == "local" else 1 | |||
| self.meta_name_bool = ( | |||
There was a problem hiding this comment.
Not really sure what meta_name_bool stands for, could we come up with some more clear naming? For example meta_use_df_name or something similar?
There was a problem hiding this comment.
I agree, for me its also not really clear what meta_name is referring to.
There was a problem hiding this comment.
Thanks, I agree!
What about:
- meta_used_in_forward
- model_uses_meta
?
There was a problem hiding this comment.
What exactly is meta even referring to? Its the name of the local dataset in case global / local modelling is used, correct?
There was a problem hiding this comment.
Yes, the dataloader has meta in it, including the 'ID' of the local dataset.
When doing global/local modeling we need to know the 'ID' of each sample on the batch.
There was a problem hiding this comment.
changed it to meta_used_in_model - Didn't add "name" on it as this could be used as well once we have static covariates, for example
There was a problem hiding this comment.
Thanks for the improvement @alfonsogarciadecorral - I find the meta naming of this functionality rather unclear, but that is not related to your work - I'll open a follow up issue on that.
Co-authored-by: Richard Stromer <noxan@users.noreply.github.com>
@ourownstory
This simple PR is to reduce some lines the global-local logic.
A boolean variable called
meta_name_boolwill be used if we need to know the time series ID when we interact with the Model.Which is the best release I should add this to?