Skip to content
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

Fix some bugs #1174

Merged
merged 27 commits into from
Oct 17, 2023
Merged

Fix some bugs #1174

merged 27 commits into from
Oct 17, 2023

Conversation

kasyanovse
Copy link
Collaborator

@kasyanovse kasyanovse commented Sep 25, 2023

Test does not pass until aimclub/GOLEM#202 is not in release.

  1. Fix Unserializable operation #1173 + new test
  2. Fix Fedot cannot interpret n_jobs < -1 #1170 + new test (+ Add checks to determine_n_jobs and add test for it GOLEM#202)
  3. Fix Lagged returns 1-dimensional target for forecast length = 1 #1159
  4. Add test for Investigate performance for new data operations #262
  5. Add verbose=0 for boosting models
  6. Raise error if window_size is incorrect in LaggedImplementation.
  7. Set non-default tag for CGRU and ransac_non_lin_reg due to high evaluation time.
  8. Set non-default tag for CutImplementation due to specific type of operation.
  9. Fix second point from Todo for pred_ints #1165
  10. Set non-default tag for ExtraTreesRegressor and gbr due to more effective alternatives.

@kasyanovse kasyanovse linked an issue Sep 25, 2023 that may be closed by this pull request
@aim-pep8-bot
Copy link

aim-pep8-bot commented Sep 25, 2023

Hello @kasyanovse! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 78:9: E722 do not use bare 'except'

Comment last updated at 2023-09-28 14:14:44 UTC

@kasyanovse kasyanovse linked an issue Sep 25, 2023 that may be closed by this pull request
@pep8speaks
Copy link

pep8speaks commented Sep 25, 2023

Hello @kasyanovse! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 78:9: E722 do not use bare 'except'

Comment last updated at 2023-09-28 14:14:46 UTC

@kasyanovse kasyanovse linked an issue Sep 25, 2023 that may be closed by this pull request
@kasyanovse kasyanovse self-assigned this Sep 25, 2023
@kasyanovse kasyanovse added the bug Something isn't working label Sep 28, 2023
@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Merging #1174 (f92cbfa) into master (17b2ecd) will decrease coverage by 0.36%.
Report is 3 commits behind head on master.
The diff coverage is 95.23%.

@@            Coverage Diff             @@
##           master    #1174      +/-   ##
==========================================
- Coverage   79.70%   79.35%   -0.36%     
==========================================
  Files         141      141              
  Lines        9851     9784      -67     
==========================================
- Hits         7852     7764      -88     
- Misses       1999     2020      +21     
Files Coverage Δ
...edot/core/pipelines/prediction_intervals/params.py 100.00% <100.00%> (ø)
...ion_intervals/solvers/mutation_of_best_pipeline.py 81.96% <100.00%> (+0.93%) ⬆️
...core/pipelines/prediction_intervals/ts_mutation.py 97.22% <100.00%> (-0.15%) ⬇️
fedot/core/pipelines/prediction_intervals/utils.py 68.42% <ø> (-1.10%) ⬇️
...lementations/data_operations/ts_transformations.py 89.10% <85.71%> (-0.54%) ⬇️

... and 18 files with indirect coverage changes

window_size = len(time_series) - forecast_length - 10
self.params.update(window_size=window_size)
self.log.info(f"{prefix} from {previous_size} to {self.window_size}.")
if self.window_size + forecast_length > len(time_series):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Что предполагается делать в случае, когда длина окна больше допустимой для конкретного ряда?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кидать ошибку. Это некорректный случай.
Идея в том, чтобы убрать скрытое поведение. Если тюнер или композер хотят поставить некорректное значение окна, то пусть они узнают об этом.

forecast_length = input_data.task.task_params.forecast_length
data_length = input_data.features.shape[0]
for node in pipeline_for_forecast.nodes:
if node.name == 'lagged':
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Мне кажется, что это должно учитываться где-то внутри. Случайный пользователь не додумается до такого

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так это и есть внутри. Разве нет?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сейчас получается так, что, если у lagged преобразования длина окна слишком большая для конкретного ряда, то вываливается ошибка, которая нигде не обрабатывается. Это код только для замены пропусков, длина окна будет подбираться только для этой задачи, а в общем случае будет ошибка. То есть придется в любой код, в котором мы не хотим, чтобы вываливалась ошибка, писать подобные условия

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Все правильно, разве нет?
Если длина окна задана некорректно, то мы получаем ошибку. Это как с пайплайном. Сейчас некорректные начальные пайплайны вываливают ошибку, а не сами как-то корректируются.

fedot/utilities/ts_gapfilling.py Show resolved Hide resolved
test/integration/models/test_model.py Show resolved Hide resolved
simple_pipeline = get_simple_ts_pipeline(window_size=2)

max_window = int(time_series.features.shape[0] / (folds + 1)) - (forecast_len * validation_blocks) - 1
ppl_ss = PipelineSearchSpace({'lagged': {'window_size': {'hyperopt-dist': hp.uniformint,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Есть ли смысл явно ограничивать пространство?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да. Там очень небольшое количество тестовых данных, тюнер легко вылетает в область недопустимых окон и все падает, так как lagged выкидывает ошибку о недопустимости размера окна.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А как сейчас эта ошибка вообще обрабатывается?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тюнером?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Например. Или композером

Copy link
Collaborator Author

@kasyanovse kasyanovse Oct 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

С композером проблем нет потому, что оценка пайплайна обернута в try-except, пайплайн с некорректным окном получается некорректным. Тюнер вылетает. Теперь вот не знаю, откатить это изменение или скорректировать тюнер.

Copy link
Collaborator

@valer1435 valer1435 Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А можно ли тюнер обернуть в try except? (в качестве временного фикса)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Починю тюнер тогда.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тюнер в починке не нуждается. Он корректно отрабатывает ошибки.

fedot/utilities/ts_gapfilling.py Show resolved Hide resolved
test/integration/models/test_model.py Show resolved Hide resolved
simple_pipeline = get_simple_ts_pipeline(window_size=2)

max_window = int(time_series.features.shape[0] / (folds + 1)) - (forecast_len * validation_blocks) - 1
ppl_ss = PipelineSearchSpace({'lagged': {'window_size': {'hyperopt-dist': hp.uniformint,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А как сейчас эта ошибка вообще обрабатывается?

@kasyanovse kasyanovse merged commit a5a9b17 into master Oct 17, 2023
5 checks passed
@kasyanovse kasyanovse deleted the 1170-fix branch October 17, 2023 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
5 participants