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.

Sign up for GitHub

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

Jump to bottom

fix: do not launch composition for atomized model #1324

Merged
merged 9 commits into from
Aug 26, 2024
Merged

fix: do not launch composition for atomized model #1324

merged 9 commits into from
Aug 26, 2024

Conversation

DRMPN
Copy link
Collaborator

@DRMPN DRMPN commented Aug 23, 2024

This is a 🐛 bug fix.

Summary

Quick Fix: prevent launch pipeline composition for Atomized model.

Example notebook with Atomized Model works as expected:
https://github.com/aimclub/FEDOT/blob/master/examples/advanced/additional_learning.py

Context

Fixes #1317

@DRMPN DRMPN added bug Something isn't working api Anything related to user-facing interfaces & parameter passing labels Aug 23, 2024
@DRMPN DRMPN self-assigned this Aug 23, 2024
@pep8speaks
Copy link

pep8speaks commented Aug 23, 2024 •
edited
Loading

Hello @DRMPN! 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-08-26 21:06:36 UTC

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 23, 2024 •
edited
Loading

All PEP8 errors has been fixed, thanks ❤️

Comment last updated at

@DRMPN
Copy link
Collaborator Author

DRMPN commented Aug 23, 2024

/fix-pep8

@codecov Codecov
Copy link

codecov bot commented Aug 23, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 80.12%. Comparing base (17b9593) to head (d6555da).
Report is 2 commits behind head on master.

Files Patch % Lines
fedot/api/main.py 60.00% 2 Missing ⚠️
fedot/api/api_utils/predefined_model.py 66.66% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1324      +/-   ##
==========================================
- Coverage   80.16%   80.12%   -0.04%     
==========================================
  Files         146      146              
  Lines       10278    10285       +7     
==========================================
+ Hits         8239     8241       +2     
- Misses       2039     2044       +5

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

Lopa10ko
Lopa10ko requested changes Aug 23, 2024
View reviewed changes
Copy link
Collaborator

@Lopa10ko Lopa10ko left a comment

Choose a reason for hiding this comment

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

не совсем понятна идея, почему для atomized пропускаем верификацию пайплайна и композицию

fedot/api/api_utils/predefined_model.py Outdated Show resolved Hide resolved
fedot/api/main.py Outdated Show resolved Hide resolved
fedot/api/api_utils/predefined_model.py Outdated Show resolved Hide resolved
fedot/api/main.py Outdated Show resolved Hide resolved
@DRMPN
Copy link
Collaborator Author

DRMPN commented Aug 23, 2024

не совсем понятна идея, почему для atomized пропускаем верификацию пайплайна и композицию

atomized функциональность на данный момент не приоритетная. Реализация будет написана позже.
Пока можно пропустить композицию.
При верификации тип модели проверяется для Голема, там Atomized Model нет, поэтому код падает.

@Lopa10ko
Copy link
Collaborator

не совсем понятна идея, почему для atomized пропускаем верификацию пайплайна и композицию

atomized функциональность на данный момент не приоритетная. Реализация будет написана позже. Пока можно пропустить композицию. При верификации тип модели проверяется для Голема, там Atomized Model нет, поэтому код падает.

понятно, нужно будет в таком случае создать дополнительно 2 issue:

  • в продолжение Atomized model operation #1227, чтобы наконец убрать AtomizedModel
  • в поддержку новой Atomized для GOLEM

@DRMPN
Copy link
Collaborator Author

DRMPN commented Aug 24, 2024

  • в продолжение Atomized model operation #1227, чтобы наконец убрать AtomizedModel
  • в поддержку новой Atomized для GOLEM

Фича с AtomizedModel в текущем виде вообще не нужна?

@DRMPN
Copy link
Collaborator Author

DRMPN commented Aug 24, 2024

/fix-pep8

@Lopa10ko
Copy link
Collaborator

Фича с AtomizedModel в текущем виде вообще не нужна?

нужна, пусть и не работает. имел в виду, что после реализации Atomized нужно будет почистить упоминания AtomizedModel

@DRMPN
Copy link
Collaborator Author

DRMPN commented Aug 26, 2024

Интеграционные: https://github.com/aimclub/FEDOT/actions/runs/10551401207

nicl-nno
nicl-nno reviewed Aug 26, 2024
View reviewed changes
fedot/api/main.py Outdated
@@ -166,6 +166,11 @@ def fit(self,
with fedot_composer_timer.launch_preprocessing():
self.train_data = self.data_processor.fit_transform(self.train_data)

init_asm = self.params.data.get('initial_assumption')
if (predefined_model is None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Тут скобки лишние.

@nicl-nno
Copy link
Collaborator

Как временный воркэраунд норм, но нужно все эти хаки пометить TODO и сообщением для пользователя.
Т,к. сейчас довольно неочевидное поведение возникает.

@DRMPN DRMPN mentioned this pull request Aug 26, 2024
Open
@DRMPN DRMPN merged commit 8bc76c2 into master Aug 26, 2024
7 checks passed
@DRMPN DRMPN deleted the DRMPN-fix-atomized-model branch August 26, 2024 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicl-nno nicl-nno nicl-nno approved these changes

@Lopa10ko Lopa10ko Lopa10ko approved these changes

Assignees

@DRMPN DRMPN

Labels
api Anything related to user-facing interfaces & parameter passing bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: AtomizedModel - AttributeError: 'NoneType' object has no attribute 'task_type'
4 participants
@DRMPN @pep8speaks @Lopa10ko @nicl-nno