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

PyO3과련 기존 레이아웃 변경 Rollback 논의: python/furiosa/models --> furiosa/models #25

Closed
hyeokjunejeon opened this issue Aug 19, 2022 · 13 comments · Fixed by #26
Assignees

Comments

@hyeokjunejeon
Copy link
Contributor

hyeokjunejeon commented Aug 19, 2022

@furiosamg 님, @hyunsik 님,

제가 테스트 해본 결과
기존 레이아웃 방식이 잘 진행되고 테스트도 잘 되었습니다.
예를들면, furiosa.init.py 를 생성하지 않아도 됩니다.

PyO3 문서에 따르면(https://maturin.rs/project_layout.html#alternate-python-source-directory-src-layout)가 python 폴더를 만들라는 것도 아닌 듯합니다.

[package]
name = "furiosa-models"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[lib]
name = "furiosa_models_native"
crate-type = ["cdylib"]

# https://maturin.rs/project_layout.html#alternate-python-source-directory-src-layout
[package.metadata.maturin]
python-source = "furiosa/models"
name = "furiosa.models.furiosa_models_native"

[dependencies]
pyo3 = { version = "0.16.5", features = ["extension-module"] }

Test code

pip install -e .

from furiosa.models import sum_as_string
sum_as_string(1,2)

Originally posted by @hyeokjunejeon in #10 (comment)

@hyeokjunejeon hyeokjunejeon changed the title PyO3 기존 레이아웃 변경 논의 PyO3 기존 레이아웃 변경 Rollback 논의 Aug 19, 2022
@furiosamg
Copy link
Collaborator

furiosamg commented Aug 19, 2022

혹시 furiosa-models-experimental 디렉토리가 아닌 곳에서도 furiosa.models가 잘 import 되시나요? 기존 레이아웃에서는 설치가 되고 설치된 것이 import가 되는 것이 아니라 상대경로로 직접 import가 되었던 것으로 기억해서요.

P.S. python 폴더 밑에 프로젝트를 두는 것은 위와 같이 실제로 깔리지 않았는데 import가 되는 상황을 구별할수 있다는 점 때문에 권장되고 있는 것입니다.

@hyeokjunejeon hyeokjunejeon changed the title PyO3 기존 레이아웃 변경 Rollback 논의 PyO3과련 기존 레이아웃 변경 Rollback 논의: python/furiosa/models --> furiosa/models Aug 19, 2022
@hyeokjunejeon
Copy link
Contributor Author

"pip install -e ."이후 furiosa.models.furiosa_models_native.so가 빌드이후 python/furiosa/models 로 옮겨진다는 거군요. 제가 조금 더 확인해 보겠습니다.

@furiosamg
Copy link
Collaborator

네네 pip install -e .이나 pip install . 이후 다른 디렉토리 (cd .. 등을 한 뒤)에 import furiosa.models 가 되는지 확인해주시면 감사하겠습니다. 제 경우엔 그게 안 됐었습니다 ㅜㅜ

@hyeokjunejeon
Copy link
Contributor Author

hyeokjunejeon commented Aug 19, 2022

네, 제가 해본 스텝은 다음과 같습니다.

저의 폴더 구조입니다.
스크린샷 2022-08-19 오후 9 23 03

Cargo.toml 설정 파일입니다.

[package]
name = "furiosa-models"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[lib]
name = "furiosa_models_native"
crate-type = ["cdylib"]

# https://maturin.rs/project_layout.html#alternate-python-source-directory-src-layout
[package.metadata.maturin]
python-source = "furiosa"
name = "models.furiosa_models_native"

[dependencies]
pyo3 = { version = "0.16.5", features = ["extension-module"] }

pip install -e . 설치

다음의 python script 실행

import furiosa.models
from furiosa.models import sum_as_string
sum_as_string(1,2)

python-source의 하위에 모든 폴더가 __init__.py를 가져하는데, 이것은 furiosa.__init__.py를 만들게 되고, import furiosa.common 관련 에러가 발생을 하였습니다.(언급하신 내용대로). 그래서 python-source=furiosa 로하면 회피는 가능해집니다.

또한 rm -f __init__.py를 하고, git push 하기전 다시 touch __init__.py 생성하고, 안되면 다시 지우고, 다시 생성하고 푸시하고를 무한 반복을 회피할 수 있게 되는 듯 합니다.

@furiosamg
Copy link
Collaborator

furiosamg commented Aug 19, 2022

저도 혁준님이 구성하신대로 구성하고 실험해 보았습니다. 저는 furiosa.models가 설치되지 않았는데요ㅜㅜ. 혹시 아래의 스크립트대로 실행해 주실 수 있으실까요?

pip install -e .
cd ..
python -c "import furiosa.models"

즉 아래 스크립트를 실행하기 전에 cd .. 을 실행해 주시고 실행해봐주셨으면 합니다.

import furiosa.models
from furiosa.models import sum_as_string
sum_as_string(1,2)

실제로 설치된 패키지는 models입니다. 그래서 어느 위치에서든

import models
from models import sum_as_string
sum_as_string(1,2)

코드는 작동하게 됩니다.

원래의 프로젝트 구조에서는 furiosa.models 패키지가 설치되지가 않습니다. 혁준님이 실행하신 import furiosa.models가 성공하는 이유는 설치된 furiosa.models 패키지를 불러온 것이 아니라 현재 폴더 밑에서 소스 코드를 찾고 가져와서 실행되고 있는 형태인데요, 즉 배포 가능한 패키지를 아예 생성 못하고 있는 것입니다. (실제로 python3 -c "import sysconfig; print(sysconfig.get_path('purelib'))" 명령으로 파이썬 패키지들이 설치되고 있는 site-packages를 찾아 들어가 보면 /site-packages/furiosa 밑에 models 디렉토리가 없는 것을 확인할 수 있습니다. 대신 /site-packages/models/ 밑에 소스 코드들이 존재합니다.)

#8#10 의 댓글도 함께 읽어주시면 정말 감사하겠습니다.

제가 #10 에서

Maturin의 mixed project guideline에 따라

라고 적어 현식님과 혁준님 모두에게 혼란을 드린 것 같은데요, 좀 자세히 다시 말하자면, 원래 구조에서는 패키지가 설치되지 않아도 local relative import(./furiosa/models/ 밑의 소스코드들)를 통해 패키지가 실제로 설치되고 잘 실행되는 것 같은 착각을 줄 수 있습니다. 그래서 이를 피하기 위해 많은 파이썬 패키지들이 ./src 디렉토리 밑에 소스 코드를 저장하곤 합니다.

근데 rust 프로젝트도 src 밑에 소스 코드들을 저장하는데요, 파이썬과 러스트 소스코드가 모두 src 밑에 저장되면 어느 파일이 어느 프로젝트에 속하는지 모르게 되어 동작을 안 하게 됩니다.

근데 파이썬과 러스트가 다른 점이 있습니다. 러스트는 소스 코드가 꼭 src 디렉토리 밑에 있기를 기대하는 반면, 파이썬은 디렉토리 이름이 꼭 src가 아니어도 됩니다. 그래서 PyO3에서 python 소스 코드들을 python 디렉토리 밑에 두는 예시를 만들어 둔 것으로 보입니다.

참고할만한 이슈: PyO3/maturin#490

@hyeokjunejeon
Copy link
Contributor Author

hyeokjunejeon commented Aug 19, 2022

네, 상기 부분은 제가 더 테스트 해보겠습니다. 잘 안되는 점은 있는 듯 합니다.

아직도 어려운 점은 다음의 스텝입니다.
이 방법이 어렵다면, 아래의 방식을 막기 위해서는 cbox_decoders 방식(python/furiosa/models/vision/yolov5/box_decode/cbox_decode)도 좋은 듯 합니다.

개발 완료까지 다음 반복:
rm -f python/furiosa/__init__.py
작업
touch python/furiosa/__init__.py
git commit

@furiosamg
Copy link
Collaborator

네 저도 그래서 더 해결 방법을 찾아 보려고 하고 있습니다. ㅜㅜ

@hyeokjunejeon
Copy link
Contributor Author

hyeokjunejeon commented Aug 19, 2022

네, 고생하십니다.
제가 방금 성공한 안은 어떤가요?

-> python/ 밑에 구조는 조금 전 저의 실수를 막기 위해선, 좋은 구조일 듯 합니다.

-> 아직 maturin은 성숙도가 높지 않은 듯합니다. 그래서 고전적이지만, 그나마 온갖 수난을 겪인 setuptools-rust 를 하는 것을 제안드려 봅니다. 추후에 maturin의 성숙도가 높았을때 해도 목적 달성에는 큰 지장이 없을 듯 합니다.
성공한 시나리오 입니다.


MANIFEST.in

include Cargo.toml
recursive-include src *
recursive-include python/furiosa/models *

setup.py

from setuptools import find_namespace_packages, setup
from setuptools_rust import Binding, RustExtension

setup(
    name="furiosa-models",
    version="1.0",
    packages=["furiosa.models"],
    package_dir={'': 'python'},
    # packages=find_namespace_packages(include=["furiosa.models.*"]),
    rust_extensions=[
        RustExtension(
            "furiosa.models.furiosa_models_native",
            path="Cargo.toml",
            binding=Binding.PyO3,
            debug=False,
        )
    ],
    # rust extensions are not zip safe, just like C-extensions.
    zip_safe=False,
)

Cargo.toml

[package]
name = "furiosa-models"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[lib]
name = "furiosa_models_native"
crate-type = ["cdylib"]

# https://maturin.rs/project_layout.html#alternate-python-source-directory-src-layout
#[package.metadata.maturin]
#python-source = "furiosa"
#name = "models.furiosa_models_native"

[dependencies]
pyo3 = { version = "0.16.5", features = ["extension-module"] }

pyproject.toml

[build-system]
#requires = ["maturin>=0.13,<0.14"]
#build-backend = "maturin"
requires = ["setuptools", "wheel", "setuptools-rust"]


[project]
name = "furiosa-models"
version = "0.0.1"
authors = [{ name = "FurioaAI Inc.", email = "pkg@furiosa.ai" }]
readme = "README.md"
license = { file = "LICENSE" }
classifiers = [
    "Development Status :: 3 - Alpha",
    "Environment :: Console",
    "Environment :: Web Environment",
    "Intended Audience :: Developers",
    "Intended Audience :: System Administrators",
    "License :: OSI Approved :: Apache Software License",
    "Programming Language :: Rust",
    "Programming Language :: Python :: Implementation :: CPython",
    "Programming Language :: Python :: Implementation :: PyPy",
    "Programming Language :: Python :: 3",
    "Programming Language :: Python :: 3.7",
    "Programming Language :: Python :: 3.8",
    "Programming Language :: Python :: 3.9",
    "Topic :: Software Development :: Libraries :: Python Modules",
    "Topic :: Scientific/Engineering :: Artificial Intelligence",
]
description = "Furiosa Models"
requires-python = "~=3.7"
dependencies = [
    "furiosa-registry == 0.7.*",
    "furiosa-common == 0.7.*",

    "onnx",
    "onnxruntime",
    "opencv-python-headless",
    "pycocotools",
    "timm",
    "torch",
    "torchvision",
    "dvc[s3]",
    "pydantic",
    # Protobuf major version change issue: https://github.com/furiosa-ai/furiosa-artifacts/issues/23
    "protobuf < 4.0dev",

    "segmentation_models_pytorch",
    "pretrainedmodels",
    "effdet",

    "cbox_decode @ git+https://github.com/furiosa-ai/furiosa-artifacts.git#subdirectory=furiosa/models/vision/yolov5/box_decode/cbox_decode"
]

[project.optional-dependencies]
test = [
    "furiosa-runtime == 0.7.*",

    "pytest",
    "pytest-asyncio ~= 0.17.2",
]

[project.urls]
Home = "https://furiosa.ai"
Documentation = "https://github.com/furiosa-ai/furiosa-artifacts"
"Bug Tracker" = "https://github.com/furiosa-ai/furiosa-artifacts/issues"
"Source Code" = "https://github.com/furiosa-ai/furiosa-artifacts"

[tool.black]
skip-string-normalization = true
line-length = 100
target-version = ["py37", "py38", "py39"]
extend-exclude = "/generated/"

[tool.isort]
force_sort_within_sections = true
known_first_party = ["furiosa"]
line_length = 100
profile = "black"
extend_skip_glob = ["**/generated/**"]

[tool.pytest.ini_options]
asyncio_mode = "strict"
pythonpath = ["."]

최종 테스트

test_install.sh

#!/bin/bash
pip uninstall furiosa-models -y
rm -f ./python/furiosa/models/furiosa_models_native*.so
pip install -e .    # or pip install .

cd ..

test_furiosa_models.py

from furiosa.models import sum_as_string
import furiosa.models.vision

print(sum_as_string(1,2))

@hyunsik
Copy link
Member

hyunsik commented Aug 19, 2022

저는 좋은데요. @furiosamg 님도 좋으시면 PR 올려주시면 바로 머지하겠습니다.

@furiosamg
Copy link
Collaborator

네 maturin을 안 쓰는 것이 좋은 방법일 것 같습니다. 조사해주시고 알아봐 주셔서 정말 감사합니다. 이런 일을 겪다보니 파이썬의 패키징 관리가 얼마나 엉망인지, principle없이 기능 추가하면서 개발하면 어떤 결과가 나오는지 등 깨우친 바가 많네요..

@furiosamg
Copy link
Collaborator

혁준님이 휴가셔서 제가 PR 열겠습니다. 혁준님이 작업 결과공유주셔서 금방 할 것 같습니다 :)

@chris-ha458
Copy link

이미 1년전 close된 이슈고 프로젝트 진전 등 고려하면 바뀔가능성이 없어보이기는 하지만,
비슷한 난관을 상당한 시간이 흐른뒤에 직면한 상태라 참고하실수있도록 몇가지 디테일을 기록해둡니다.
위에 이미 PyO3/maturin#490 언급되었다시피 퓨리오사 팀 외에도 pyo3, maturin 사용하는 많은 개발자들이 비슷한 난관을 겪고 각자 다른 해결책을 찾은것 같습니다.
퓨리오사팀께서는 제가 깊게 파악하지 못한 고유의 필요성이 대두되어 결국 setup-tools로 방향을 잡으신것 같네요.

그사이에 maturin은 python하위 폴더를 사용하는것으로 방향성을 잡아 통일하였고 setuptools-rust측과도 연계하여 그런 방향으로 자리잡아가는것 같습니다.
아래는 PyO3/maturin/setuptools-rust 개발자가 올린 PR입니다.
PyO3/setuptools-rust#350

maturin init --mixed --bindings pyo3 실행시
Cargo.toml pyproject.toml python src 이게 기본구조고 python에 python 관련 src에 rust관련 내용 넣어서만들도록 안내되어있습니다.

@furiosamg
Copy link
Collaborator

이미 1년전 close된 이슈고 프로젝트 진전 등 고려하면 바뀔가능성이 없어보이기는 하지만, 비슷한 난관을 상당한 시간이 흐른뒤에 직면한 상태라 참고하실수있도록 몇가지 디테일을 기록해둡니다. 위에 이미 PyO3/maturin#490 언급되었다시피 퓨리오사 팀 외에도 pyo3, maturin 사용하는 많은 개발자들이 비슷한 난관을 겪고 각자 다른 해결책을 찾은것 같습니다. 퓨리오사팀께서는 제가 깊게 파악하지 못한 고유의 필요성이 대두되어 결국 setup-tools로 방향을 잡으신것 같네요.

그사이에 maturin은 python하위 폴더를 사용하는것으로 방향성을 잡아 통일하였고 setuptools-rust측과도 연계하여 그런 방향으로 자리잡아가는것 같습니다. 아래는 PyO3/maturin/setuptools-rust 개발자가 올린 PR입니다. PyO3/setuptools-rust#350

maturin init --mixed --bindings pyo3 실행시 Cargo.toml pyproject.toml python src 이게 기본구조고 python에 python 관련 src에 rust관련 내용 넣어서만들도록 안내되어있습니다.

안녕하세요 먼저 저희 프로젝트에 관심 가져주셔서 감사합니다! :)
이 이슈는 Python 소스 코드를 /python 밑에 둘 지 루트에 바로 둘 지에 관한 이슈인데요, 이 이슈 때문보다는 저희는 PyO3/maturin#811 native namespace package 지원 문제 때문에 다른 패키지 매니저를 쓰는 방향으로 가게 되었습니다.

다시 한번 관심 감사드립니다!

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 a pull request may close this issue.

4 participants