Autogluon timeseries, addressed comments by sebhrusen #7
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Innixma autogluon timeseries
Innixma
approved these changes
Oct 6, 2022
| @@ -129,6 +138,38 @@ def __repr__(self): | |||
| return repr_def(self) | |||
|
|
|||
|
|
|||
| def extend_dataset_with_timeseries_config(self, dataset, dataset_config): | |||
| if dataset_config['id_column'] is None: | |||
| log.warning("Warning: For timeseries task setting undefined itemid column to `item_id`.") | |||
There was a problem hiding this comment.
refer to it in the warning with the correct key 'id_column'
| @@ -129,6 +138,38 @@ def __repr__(self): | |||
| return repr_def(self) | |||
|
|
|||
|
|
|||
| def extend_dataset_with_timeseries_config(self, dataset, dataset_config): | |||
There was a problem hiding this comment.
we are manipulating the outer context of dataset_config, maybe this is ok but want to mention that it is happening.
There was a problem hiding this comment.
Same with manipulating outer context of dataset. Consider adding documentation stating that this is intended or otherwise do a deep copy on the objects.
Comment on lines
+14
to
+50
| if dataset.type is not DatasetType.timeseries: | ||
|
|
||
| data = dict( | ||
| train=dict(path=dataset.train.data_path('parquet')), | ||
| test=dict(path=dataset.test.data_path('parquet')), | ||
| target=dict( | ||
| name=dataset.target.name, | ||
| classes=dataset.target.values | ||
| ), | ||
| problem_type=dataset.type.name # AutoGluon problem_type is using same names as amlb.data.DatasetType | ||
| ) | ||
| exec_file = "exec.py" | ||
|
|
||
| else: | ||
| dataset = deepcopy(dataset) | ||
| if not hasattr(dataset, 'timestamp_column'): | ||
| dataset.timestamp_column = None | ||
| if not hasattr(dataset, 'id_column'): | ||
| dataset.id_column = None | ||
| if not hasattr(dataset, 'forecast_range_in_steps'): | ||
| raise AttributeError("Unspecified `forecast_range_in_steps`.") | ||
|
|
||
| data = dict( | ||
| # train=dict(path=dataset.train.data_path('parquet')), | ||
| # test=dict(path=dataset.test.data_path('parquet')), | ||
| train=dict(path=dataset.train.path), | ||
| test=dict(path=dataset.test.path), | ||
| target=dict( | ||
| name=dataset.target.name, | ||
| classes=dataset.target.values | ||
| ), | ||
| problem_type=dataset.type.name, # AutoGluon problem_type is using same names as amlb.data.DatasetType | ||
| timestamp_column=dataset.timestamp_column, | ||
| id_column=dataset.id_column, | ||
| forecast_range_in_steps=dataset.forecast_range_in_steps | ||
| ) | ||
| exec_file = "exec_ts.py" |
There was a problem hiding this comment.
if/else could be better broken up into dedicated functions for each modality to avoid an overly long function with a bunch of if/elif/elif/elif/else in future
Innixma
added a commit
that referenced
this pull request
Oct 26, 2022
* Add AutoGluon TimeSeries Prototype * AutoMLBenchmark TimeSeries Prototype. (#6) * fixed loading test & train, changed pred.-l. 5->30 * ignore launch.json of vscode * ensuring timestamp parsing * pass config, save pred, add results * remove unused code * add readability, remove slice from timer * ensure autogluonts has required info * add comments for readability * setting defaults for timeseries task * remove outer context manipulation * corrected spelling error for quantiles * adding mape, correct available metrics * beautify config options * fixed config for public access * Update readme * Autogluon timeseries, addressed comments by sebhrusen (#7) * fixed loading test & train, changed pred.-l. 5->30 * ignore launch.json of vscode * ensuring timestamp parsing * pass config, save pred, add results * remove unused code * add readability, remove slice from timer * ensure autogluonts has required info * add comments for readability * setting defaults for timeseries task * remove outer context manipulation * corrected spelling error for quantiles * adding mape, correct available metrics * beautify config options * fixed config for public access * no outer context manipulation, add dataset subdir * add more datasets * include error raising for too large pred. length. * mergin AutoGluonTS framework folder into AutoGluon * renaming ts.yaml to timeseries.yaml, plus ext. * removing presets, correct latest config for AGTS * move dataset timeseries ext to datasets/file.py * dont bypass test mode * move quantiles and y_past_period_error to opt_cols * remove whitespaces * deleting merge artifacts * delete merge artifacts * renaming prediction_length to forecast_range_in_steps * use public dataset, reduced range to maximum * fix format string works * fix key error bug, remove magic time limit * Addressed minor comments, and fixed version call for tabular and timeseries modularities (#8) * fixed loading test & train, changed pred.-l. 5->30 * ignore launch.json of vscode * ensuring timestamp parsing * pass config, save pred, add results * remove unused code * add readability, remove slice from timer * ensure autogluonts has required info * add comments for readability * setting defaults for timeseries task * remove outer context manipulation * corrected spelling error for quantiles * adding mape, correct available metrics * beautify config options * fixed config for public access * no outer context manipulation, add dataset subdir * add more datasets * include error raising for too large pred. length. * mergin AutoGluonTS framework folder into AutoGluon * renaming ts.yaml to timeseries.yaml, plus ext. * removing presets, correct latest config for AGTS * move dataset timeseries ext to datasets/file.py * dont bypass test mode * move quantiles and y_past_period_error to opt_cols * remove whitespaces * deleting merge artifacts * delete merge artifacts * renaming prediction_length to forecast_range_in_steps * use public dataset, reduced range to maximum * fix format string works * fix key error bug, remove magic time limit * swapped timeseries and tabular to set version * make warning message more explicit * remove outer context manipulation * split timeseries / tabular into functions Co-authored-by: Leo <LeonhardSommer96@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addressed all comments by sebhrusen. Except for the renaming of prediction length.