-
Notifications
You must be signed in to change notification settings - Fork 289
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
Score fixed #485
base: development
Are you sure you want to change the base?
Score fixed #485
Conversation
Codecov ReportBase: 85.49% // Head: 84.50% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## development #485 +/- ##
===============================================
- Coverage 85.49% 84.50% -0.99%
===============================================
Files 231 232 +1
Lines 16351 16475 +124
Branches 3028 2734 -294
===============================================
- Hits 13979 13923 -56
- Misses 1533 1675 +142
- Partials 839 877 +38
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
autoPyTorch/api/base_task.py
Outdated
@@ -1791,6 +1792,8 @@ def score( | |||
The test predictions | |||
y_test (np.ndarray): | |||
The test ground truth labels. | |||
score_kwargs: Any | |||
additional arguments for computing the scores. Some metrics might require special arguments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you add a note saying something like: Currently, needed for time series forecasting tasks. See autoPyTorch/api/time_series_forecasting.py
if isinstance(seasonality, list): | ||
seasonality = min(seasonality) # Use to calculate MASE | ||
self.seasonality = int(seasonality) # type: ignore[call-overload] | ||
self.seasonality = self.datamanager.seasonality |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the AbstractEvaluator
class, we use a _init_datamanager_info
(see here). At the end of this method we delete the datamaneger object to reduce the memory usage. Could you also update the time series forecasting evaluator, maybe overriding the parent class function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can also do it as a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data manager is required somewhere else by the time series evaluator because I defined time series dataset's getitem differently. Maybe in the future, we could fix that. But currently, I would prefer to keep self.datamanager in the evaluator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your changes. I think the tests are failing because we need to update the requirements. Particularly, gluonts version 0.11.4
does not have 'DayOfMonth' from 'gluonts.time_feature'. Could you take a look?
This is fixed. However, I am considering removing all these dependencies (gluonts, pytorch-forecasting) in the future (which might take some time, though). |
Types of changes
Note that a Pull Request should only contain one of refactoring, new features or documentation changes.
Please separate these changes and send us individual PRs for each.
For more information on how to create a good pull request, please refer to The anatomy of a perfect pull request.
Checklist:
Description
Fix #484 to allow the forecasting-related arguments to be passed to compute
task.score
Motivation and Context
How has this been tested?