-
Notifications
You must be signed in to change notification settings - Fork 86
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
Adaptation of API for multimodal data #663
Conversation
Hello @andreygetmanov! 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 2022-07-15 09:40:51 UTC |
505ff69
to
cb8bc5c
Compare
fedot/core/operations/evaluation/operation_implementations/implementation_interfaces.py
Outdated
Show resolved
Hide resolved
fedee4c
to
e69054b
Compare
Напиши плиз сообщение в наш чат с code review - может ребята захотят поревьюить. Кто-то один точно должен захотеть И ещё от меня просьба, - раз запрошено моё code review - пожалуйста, дай мне время его посмотреть (желательно конечно, чтобы это время было предусмотрено не поздно вечером и в выходные, как это получилось с прошлым PR'м, когда все правки по сути были сделаны в период когда я ехал с работы домой и по быстрому все влито в мастер). Не вливай 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.
Стоит не забыть поправить класс MulitmodalStrategy, так как именно в нём определяются data source узлы, но нынешняя логика сильно усеченная и не использует тип данных, надо его до туда прокинуть и использовать
См. строчку https://github.com/nccr-itmo/FEDOT/blob/master/fedot/api/api_utils/data_definition.py#L137
0fc6a1f
to
5edacb7
Compare
0f77f29
to
a96a60f
Compare
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.
Попробуй ещё плиз в рамках этого PR подумать над примером examples/simple/multitask_classification_regression_api.py
, возможно уже сейчас стоит предусмотреть механизм, когда пользователь хочет решать одновременно две разных задачи (классификацию и регрессию, как в этом кейсе например)
bcc992c
to
22b2971
Compare
Codecov Report
@@ Coverage Diff @@
## master #663 +/- ##
==========================================
+ Coverage 87.71% 88.16% +0.44%
==========================================
Files 172 181 +9
Lines 12098 12467 +369
==========================================
+ Hits 10612 10991 +379
+ Misses 1486 1476 -10
Continue to review full report at Codecov.
|
54fa8b4
to
d510824
Compare
d510824
to
9b8085b
Compare
- fixed multimodal data preprocessing bug - removed duplicated categorical encoding from preprocessor
- updated CNN tests
- added some docstrings - removed multi_modal_genre_prediction.py case
- added search_space params for word2vec_pretrained and tfidf
- modified data_has_text_features function in preprocessing builder
- text vectorizer params initialisation is moved to init part of class
8145bed
to
0a22b97
Compare
0a22b97
to
dd00c9c
Compare
dd00c9c
to
7ddf0c0
Compare
f7ee2b8
to
0adccc4
Compare
Global purpose of PR - to make Fedot API work with multimodal data
Changes: