Skip to content

Conversation

@dmitryglhf
Copy link
Collaborator

This is a 🔨 code refactoring.

Summary

  • Added new conditions for training boostings with eval_set.
  • Updated multi output for boostings

Resolved issue with test_preprocessing_through_api/test_categorical_target_processed_correctly. In cases where we tried to use LightGBM, XGBoost or CatBoost an error appeared when using eval_set due to the small dataset.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2025

All PEP8 errors has been fixed, thanks ❤️

Comment last updated at Wed, 22 Jan 2025 21:10:21

@dmitryglhf
Copy link
Collaborator Author

/fix-pep8

@codecov
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

Attention: Patch coverage is 58.13953% with 18 lines in your changes missing coverage. Please review.

Project coverage is 80.13%. Comparing base (84f4ebb) to head (e0168a7).
Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
...mplementations/models/boostings_implementations.py 58.53% 17 Missing ⚠️
fedot/core/operations/evaluation/boostings.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1358      +/-   ##
==========================================
- Coverage   80.16%   80.13%   -0.03%     
==========================================
  Files         146      146              
  Lines       10492    10512      +20     
==========================================
+ Hits         8411     8424      +13     
- Misses       2081     2088       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pep8speaks
Copy link

pep8speaks commented Jan 16, 2025

Hello @dmitryglhf! 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 2025-01-22 18:09:27 UTC

@dmitryglhf dmitryglhf requested a review from nicl-nno January 17, 2025 17:03
Comment on lines 44 to 45
is_classification_task = input_data.task.task_type == TaskTypesEnum.classification
is_regression_task = input_data.task.task_type == TaskTypesEnum.regression
Copy link
Collaborator

Choose a reason for hiding this comment

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

А task_type == ts_forecasting до сюда не может дойти? В любом случае, если случай с обоими false тут некорректен, то стоит его обработать.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Исправлено: добавлена проверка на уровень выше в модуле boostings.py

del params["early_stopping_rounds"]

# Create model with updated params
if input_data.task.task_type == TaskTypesEnum.classification:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Выше уже была такая проверка, можно переиспользовать переменную.

Comment on lines 71 to 73
params = deepcopy(self.model_params)
if params.get('early_stopping_rounds'):
del params["early_stopping_rounds"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Возможно лучше вот так менять параметр, через self.params.update?

https://github.com/aimclub/FEDOT/blob/master/fedot/core/operations/evaluation/operation_implementations/data_operations/ts_transformations.py#L131

Это фиксирует то что он поменялся внутри модели.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Исправлено: исправил на self.params.update.

Comment on lines 179 to 187
train_input, eval_input = train_test_data_setup(input_data)

# Conditions for training with eval_set
is_classification_task = input_data.task.task_type == TaskTypesEnum.classification
is_regression_task = input_data.task.task_type == TaskTypesEnum.regression
all_classes_present_in_eval = (
np.unique(np.array(train_input.target)) in np.unique(np.array(eval_input.target))
)
is_using_eval_set = bool(self.params.get('use_eval_set'))
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.

Исправлено: вынес в отдельную функцию.

@dmitryglhf dmitryglhf requested a review from nicl-nno January 23, 2025 09:48
Copy link
Collaborator

@nicl-nno nicl-nno left a comment

Choose a reason for hiding this comment

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

Не забывай только помечать в комментариях какие замечания исправлены.

@dmitryglhf dmitryglhf merged commit 6dd93b2 into master Jan 23, 2025
10 checks passed
@dmitryglhf dmitryglhf deleted the boostings-implementations-eval-set-fix branch January 23, 2025 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants