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

Домашнее задание №2 #3

Merged
merged 14 commits into from Jun 6, 2021
Merged

Домашнее задание №2 #3

merged 14 commits into from Jun 6, 2021

Conversation

polikutinevgeny
Copy link
Collaborator

Что было сделано

Использовал FastAPI, конфиг через Pydantic (не знал, что такое есть, скорее всего свежая фича). Для тестов пришлось немного поизвращаться с конфигом, но в итоге все заработало. Сделал небольшую валидацию: приводим к нужным типам и проверяем, что есть нужные колонки.
Докер не оптимизировал, потому что попытка использовать alpine через 5 минут сборки окончилась неудачей и странными ошибками. Образ опубликовал, команды написал в README.

Самооценка:

  1. ветку назовите homework2, положите код в папку online_inference

  2. Оберните inference вашей модели в rest сервис(вы можете использовать как FastAPI, так и flask, другие желательно не использовать, дабы не плодить излишнего разнообразия для проверяющих), должен быть endpoint /predict (3 балла)

  3. Напишите тест для /predict (3 балла) (https://fastapi.tiangolo.com/tutorial/testing/, https://flask.palletsprojects.com/en/1.1.x/testing/)

  4. Напишите скрипт, который будет делать запросы к вашему сервису -- 2 балла

  5. Сделайте валидацию входных данных (например, порядок колонок не совпадает с трейном, типы не те и пр, в рамках вашей фантазии) (вы можете сохранить вместе с моделью доп информацию, о структуре входных данных, если это нужно) -- 3 доп балла
    https://fastapi.tiangolo.com/tutorial/handling-errors/ -- возращайте 400, в случае, если валидация не пройдена

  6. Напишите dockerfile, соберите на его основе образ и запустите локально контейнер(docker build, docker run), внутри контейнера должен запускать сервис, написанный в предущем пункте, закоммитьте его, напишите в readme корректную команду сборки (4 балл)

6) Оптимизируйте размер docker image (3 доп балла) (опишите в readme.md что вы предприняли для сокращения размера и каких результатов удалось добиться) -- https://docs.docker.com/develop/develop-images/dockerfile_best-practices/

  1. опубликуйте образ в https://hub.docker.com/, используя docker push (вам потребуется зарегистрироваться) (2 балла)

  2. напишите в readme корректные команды docker pull/run, которые должны привести к тому, что локально поднимется на inference ваша модель (1 балл)
    Убедитесь, что вы можете протыкать его скриптом из пункта 3

  3. проведите самооценку -- 1 доп балл

Итого: 19 баллов.

@polikutinevgeny polikutinevgeny requested review from a team, ghostcat404 and Mikhail-M and removed request for a team May 9, 2021 06:36
@Mikhail-M
Copy link

следовало поместить весь код online_inference в отдельную папку.

предполагается, что это независимые "репозитории", а не все в одном проекте

Copy link

@Mikhail-M Mikhail-M left a comment

Choose a reason for hiding this comment

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

<-

@polikutinevgeny
Copy link
Collaborator Author

Я вынес, но мне кажется, что стало хуже. Так как pickle не работает без исходников (не могу пайплайн загрузить), пришлось в образ все равно положить исходный проект (хоть и в виде wheel). Тесты разнёс, но conftest.py оставил в корне проекта, чтобы не дублировать fixture.

Возможно есть какие-то best practices, которые я упускаю?

@Mikhail-M
Copy link

Я вынес, но мне кажется, что стало хуже. Так как pickle не работает без исходников (не могу пайплайн загрузить), пришлось в образ все равно положить исходный проект (хоть и в виде wheel). Тесты разнёс, но conftest.py оставил в корне проекта, чтобы не дублировать fixture.

Возможно есть какие-то best practices, которые я упускаю?

Это должно быть 2 разных проекта, первый для первой дз, второй для второй.

Если между ними есть переиспользуемые части, такие как трансформер, то их нужно либо вынести в отдельный пакет, который вы будете импортить и там и там.

В рамках этой домашки можно просто скопировать нужные куски кода

Copy link

@Mikhail-M Mikhail-M left a comment

Choose a reason for hiding this comment

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

привести в вид, что первая и вторая дз независимы.

Если очень хочется можете опубликовать в pypi и установить в зависимостях у второго=)
https://realpython.com/pypi-publish-python-package/#uploading-your-package

@polikutinevgeny
Copy link
Collaborator Author

В нужный вид привёл. Так как публиковать в pypi не хотелось (не люблю мусорить), то я воспользовался bleeding edge версией cloudpickle (ещё даже не вмерджили, cloudpipe/cloudpickle#417) и сериализовал пайплайн вместе с heart_disease.

Для тестов: нужные fixture скопипастил (их было не очень много). Чтобы не тащить за собой весь heart_disease для тестов, ожидается, что артефакты будут лежать по пути в .env (возможно стоило сделать по-другому? например, создать и сохранить простой пайплайн/модель?).
Аналогично с артефактами для сборки.

@polikutinevgeny
Copy link
Collaborator Author

Я же кстати правильно понимаю, что в реальном проекте правильный способ решения проблемы с зависимостями -- это как раз публикация пакета во внутренний репозиторий и включение в зависимости?

@polikutinevgeny
Copy link
Collaborator Author

Окончательно убрал последнюю зависимость от ml_project

Copy link

@Mikhail-M Mikhail-M left a comment

Choose a reason for hiding this comment

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

Мне уже неловко, но я вижу код дз один в этом реквесте такого быть не должно

@polikutinevgeny
Copy link
Collaborator Author

Как минимум, для валидации данных пришлось сохранить метаданные, это без редактирования ДЗ 1 не сделать?

@polikutinevgeny
Copy link
Collaborator Author

Хотя в принципе понятно, как это сделать, сделаю

Copy link

@Mikhail-M Mikhail-M left a comment

Choose a reason for hiding this comment

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

Тесты: 2 балла, хорошо, чтобы модель готовилась в фикстуре или мокалось ее поведение, а не использовалась боевая
Валидация: 1 балл, не очень явная логика, когда допом еще в pd.DataFrame в этой же функции переводятся, не оч много проверок, можно написать сильно красивее, если заюзать https://pydantic-docs.helpmanual.io/usage/validators/
За оптимизацию накинем балл, так как образ использовался не стандартный, старание все же=)

Баллы: 17

PS: сделайте так, чтобы при сдаче 3-й дз не было лишнего кода

@polikutinevgeny polikutinevgeny merged commit 70059ad into main Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants