-
Notifications
You must be signed in to change notification settings - Fork 87
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
add window size selector #1237
add window size selector #1237
Conversation
Hello @kasyanovse! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2024-01-04 08:42:06 UTC |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1237 +/- ##
==========================================
+ Coverage 79.47% 79.93% +0.46%
==========================================
Files 145 145
Lines 9928 9945 +17
==========================================
+ Hits 7890 7950 +60
+ Misses 2038 1995 -43 ☔ View full report in Codecov by Sentry. |
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.
В теории ок, выглядит достаточно лаконично. Но есть вопросы:
- Сейчас подбор окна происходит при каждом вызове transform. впринципе ок, думаю можно оставить подбор для каждого фолда, но не слишеом ли то долго?
- Как-то можно посмотреть настоящую длину окна, а не параметр 0?
- Как проверял эффективность? Действительно ли стало лучше?
Почему при каждом вызове? Я сейчас посмотрел, после выбора параметры сохраняются в узле.
Да, в
#1227 в описании PR есть таблички с оценкой изменения положения пайплайна внутри поколения после мутации. |
Кстати тюнер будет игнорировать длину окна или нет? |
Не будет. Он как раз и дотюнивает окно после композиции. |
@@ -103,7 +105,7 @@ def transform_for_fit(self, input_data: InputData) -> OutputData: | |||
self._update_column_types(output_data) | |||
return output_data | |||
|
|||
def _check_and_correct_window_size(self, time_series: np.array, forecast_length: int): | |||
def _check_and_correct_window_size(self, time_series: np.ndarray, forecast_length: int): |
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.
Я про то, это этот метод вызывается в transform
и transform_for_fit
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.
Окно подстраивается только если оно по дефолту не задано. Если задать его руками или изменить в процессе композиции/тюнинга, то оно не будет переопределяться.
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.
То есть сейчас это работает так:
- Дефолтное значение длины окна 0.
- Пусть у нас 3 фолда валидации
- Заходим в первый фолд, подбираем длину окна
- На следующих фолдах и при финальном обучении на всем ряду размер окна не подбираем?
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.
Добавил тесты что при вычислении графа на фолдах и внутри тюнера окно не перенастраивается.
|
||
|
||
def test_tuner_correctly_work_with_window_size_selector(): | ||
ts = get_timeseries(length=1000, random=True) |
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.
Не много данных для теста?
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.
Чтобы автовыбор окна случайно не совпал с тюнером им нужна некоторая свобода выбора.
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.
А по времени сколько работает? Я апрувну, но если тест работает дольше 2-3 секунд, стоит урезать
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.
test_time_series_operations.py
у меня на компе за 1,5 секунды выполняется.
Method from #1186.
lagged
node.Without window size auto selection window mutation is very usefull, it ups pipeline in sorted by fitness population up to 0.2 of quantile value, but with auto selection it downs pipeline up to 0.12 of quantile value. Therefore, window tuning by mutation is bad for pipeline. For additional information see tables in : Atomized model operation #1227.