-
Notifications
You must be signed in to change notification settings - Fork 6
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
Refactor Model Zoo #166
Refactor Model Zoo #166
Conversation
b58c4c9
to
b908d6f
Compare
5f4df9b
to
2232c25
Compare
Pip freeze result for ssd-mobilenetpip freeze resultaiobotocore==2.5.4 aiofiles==23.1.0 aiohttp==3.8.5 aiohttp-retry==2.8.3 aioitertools==0.11.0 aiosignal==1.3.1 amqp==5.1.1 annotated-types==0.5.0 antlr4-python3-runtime==4.9.3 appdirs==1.4.4 async-timeout==4.0.2 asyncssh==2.13.2 atpublic==4.0 attrs==23.1.0 billiard==4.1.0 boto3==1.28.17 botocore==1.31.17 celery==5.3.1 certifi==2023.7.22 cffi==1.15.1 charset-normalizer==3.2.0 click==8.1.6 click-didyoumean==0.3.0 click-plugins==1.1.1 click-repl==0.3.0 colorama==0.4.6 configobj==5.0.8 contourpy==1.1.0 cryptography==41.0.3 cycler==0.11.0 Cython==3.0.0 dictdiffer==0.9.0 diskcache==5.6.1 distro==1.8.0 dpath==2.1.6 dulwich==0.21.5 dvc==3.13.3 dvc-data==2.12.2 dvc-http==2.30.2 dvc-objects==0.24.1 dvc-render==0.5.3 dvc-s3==2.23.0 dvc-studio-client==0.13.0 dvc-task==0.3.0 exceptiongroup==1.1.2 filelock==3.12.2 flatten-dict==0.4.2 flufl.lock==7.1.1 fonttools==4.42.0 frozenlist==1.4.0 fsspec==2023.6.0 funcy==2.0 furiosa-common==0.10.0 furiosa-models @ file:///app furiosa-native-postprocess==0.9.0.dev0 furiosa-native-runtime==0.10.0 furiosa-quantizer==0.10.0 furiosa-quantizer-impl==0.10.0 furiosa-runtime==0.10.0 furiosa-tools==0.10.0 gitdb==4.0.10 GitPython==3.1.32 grandalf==0.8 hydra-core==1.3.2 idna==3.4 importlib-resources==6.0.1 iniconfig==2.0.0 iterative-telemetry==0.0.8 jmespath==1.0.1 kiwisolver==1.4.4 kombu==5.3.1 markdown-it-py==3.0.0 matplotlib==3.7.2 mdurl==0.1.2 multidict==6.0.4 multipledispatch==1.0.0 networkx==3.1 numpy==1.25.2 nvidia-cublas-cu11==11.10.3.66 nvidia-cuda-nvrtc-cu11==11.7.99 nvidia-cuda-runtime-cu11==11.7.99 nvidia-cudnn-cu11==8.5.0.96 omegaconf==2.3.0 onnx==1.14.0 opencv-python-headless==4.8.0.74 orjson==3.9.4 packaging==23.1 pandas==2.0.3 pathspec==0.11.2 Pillow==10.0.0 platformdirs==3.10.0 pluggy==1.2.0 prompt-toolkit==3.0.39 protobuf==4.24.0 psutil==5.9.5 py-cpuinfo==9.0.0 pyarrow==12.0.1 pycocotools==2.0.6 pycparser==2.21 pydantic==2.1.1 pydantic_core==2.4.0 pydot==1.4.2 pygit2==1.12.2 Pygments==2.16.1 pygtrie==2.5.0 pyparsing==3.0.9 pytest==7.4.0 pytest-asyncio==0.17.2 pytest-benchmark==4.0.0 python-dateutil==2.8.2 pytz==2023.3 PyYAML==6.0.1 requests==2.31.0 rich==13.5.2 ruamel.yaml==0.17.32 ruamel.yaml.clib==0.2.7 s3fs==2023.6.0 s3transfer==0.6.1 scmrepo==1.1.0 shortuuid==1.0.11 shtab==1.6.4 six==1.16.0 smmap==5.0.0 sqltrie==0.7.0 tabulate==0.9.0 tomli==2.0.1 tomlkit==0.12.1 torch==1.13.1 torchvision==0.14.1 tqdm==4.65.2 typing_extensions==4.7.1 tzdata==2023.3 urllib3==1.26.16 vine==5.0.0 voluptuous==0.13.1 wcwidth==0.2.6 wrapt==1.15.0 yarl==1.9.2 zc.lockfile==3.0.post1 zipp==3.16.2 |
3853bff ci: preload libgomp in regression tests
|
48968de
to
8638e44
Compare
8638e44
to
b850c47
Compare
Pip freeze result for allpip freeze resultaiobotocore==2.5.4 aiofiles==23.2.1 aiohttp==3.8.5 aiohttp-retry==2.8.3 aioitertools==0.11.0 aiosignal==1.3.1 amqp==5.1.1 annotated-types==0.5.0 antlr4-python3-runtime==4.9.3 appdirs==1.4.4 async-timeout==4.0.3 asyncssh==2.13.2 atpublic==4.0 attrs==23.1.0 billiard==4.1.0 boto3==1.28.17 botocore==1.31.17 celery==5.3.1 certifi==2023.7.22 cffi==1.15.1 charset-normalizer==3.2.0 click==8.1.6 click-didyoumean==0.3.0 click-plugins==1.1.1 click-repl==0.3.0 cmake==3.27.1 colorama==0.4.6 configobj==5.0.8 contourpy==1.1.0 cryptography==41.0.3 cycler==0.11.0 Cython==3.0.0 dictdiffer==0.9.0 diskcache==5.6.1 distro==1.8.0 dpath==2.1.6 dulwich==0.21.5 dvc==3.15.1 dvc-data==2.13.1 dvc-http==2.30.2 dvc-objects==0.25.0 dvc-render==0.5.3 dvc-s3==2.23.0 dvc-studio-client==0.13.0 dvc-task==0.3.0 exceptiongroup==1.1.2 filelock==3.12.2 flatten-dict==0.4.2 flufl.lock==7.1.1 fonttools==4.42.0 frozenlist==1.4.0 fsspec==2023.6.0 funcy==2.0 furiosa-common==0.10.0 furiosa-models @ file:///app furiosa-native-postprocess==0.9.0.dev0 furiosa-native-runtime==0.10.0 furiosa-quantizer==0.10.0 furiosa-quantizer-impl==0.10.0 furiosa-runtime==0.10.0 furiosa-tools==0.10.0 gitdb==4.0.10 GitPython==3.1.32 grandalf==0.8 hydra-core==1.3.2 idna==3.4 importlib-resources==6.0.1 iniconfig==2.0.0 iterative-telemetry==0.0.8 Jinja2==3.1.2 jmespath==1.0.1 kiwisolver==1.4.4 kombu==5.3.1 lit==17.0.0rc2 markdown-it-py==3.0.0 MarkupSafe==2.1.3 matplotlib==3.7.2 mdurl==0.1.2 mpmath==1.3.0 multidict==6.0.4 multipledispatch==1.0.0 networkx==3.1 numpy==1.25.2 nvidia-cublas-cu11==11.10.3.66 nvidia-cuda-cupti-cu11==11.7.101 nvidia-cuda-nvrtc-cu11==11.7.99 nvidia-cuda-runtime-cu11==11.7.99 nvidia-cudnn-cu11==8.5.0.96 nvidia-cufft-cu11==10.9.0.58 nvidia-curand-cu11==10.2.10.91 nvidia-cusolver-cu11==11.4.0.1 nvidia-cusparse-cu11==11.7.4.91 nvidia-nccl-cu11==2.14.3 nvidia-nvtx-cu11==11.7.91 omegaconf==2.3.0 onnx==1.14.0 opencv-python-headless==4.8.0.76 orjson==3.9.4 packaging==23.1 pandas==2.0.3 pathspec==0.11.2 Pillow==10.0.0 platformdirs==3.10.0 pluggy==1.2.0 prompt-toolkit==3.0.39 protobuf==4.24.0 psutil==5.9.5 py-cpuinfo==9.0.0 pyarrow==12.0.1 pycocotools==2.0.6 pycparser==2.21 pydantic==2.1.1 pydantic_core==2.4.0 pydot==1.4.2 pygit2==1.12.2 Pygments==2.16.1 pygtrie==2.5.0 pyparsing==3.0.9 pytest==7.4.0 pytest-asyncio==0.17.2 pytest-benchmark==4.0.0 python-dateutil==2.8.2 pytz==2023.3 PyYAML==6.0.1 requests==2.31.0 rich==13.5.2 ruamel.yaml==0.17.32 ruamel.yaml.clib==0.2.7 s3fs==2023.6.0 s3transfer==0.6.1 scmrepo==1.2.1 shortuuid==1.0.11 shtab==1.6.4 six==1.16.0 smmap==5.0.0 sqltrie==0.7.0 sympy==1.12 tabulate==0.9.0 tomli==2.0.1 tomlkit==0.12.1 torch==2.0.1 torchvision==0.15.2 tqdm==4.66.1 triton==2.0.0 typing_extensions==4.7.1 tzdata==2023.3 urllib3==1.26.16 vine==5.0.0 voluptuous==0.13.1 wcwidth==0.2.6 wrapt==1.15.0 yarl==1.9.2 zc.lockfile==3.0.post1 zipp==3.16.2 |
b850c47 fix: address reviews from @senokay
|
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.
전체적으로 훑어보았습니다.
이미 많은 개선을 해 주셨는데요, 사소한 부분들에 커멘트를 남겼습니다.
확인 부탁드립니다. 감사합니다.
docs/examples/ssd_mobilenet.py
Outdated
mobilenet = SSDMobileNet.load() | ||
with session.create(mobilenet.enf) as sess: | ||
mobilenet = SSDMobileNet() | ||
with create_runner(mobilenet.model_source()) as create_runner: |
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.
다른 코드들과 일관성을 맞추면 좋을 것 같습니다.
with create_runner(mobilenet.model_source()) as create_runner: | |
with create_runner(mobilenet.model_source()) as runner: |
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.
dc0aecc 에서 반영했습니다.
furiosa/models/data/enf_generator.py
Outdated
editor = ModelEditor(onnx_model) | ||
for input_name in get_pure_input_names(onnx_model): | ||
editor.convert_input_type(input_name, TensorType.UINT8) | ||
dfg = quantize(onnx_model, calib_ranges) |
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.
다른 부분에서는 quantized_onnx
로 변경을 하셨던데 여기는 아직 dfg
로 남아있습니다.
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.
감사합니다! dc0aecc 에서 반영했습니다.
|
||
image = ["tests/assets/cat.jpg"] | ||
|
||
mobilenet = SSDMobileNet.load(use_native=True) | ||
with session.create(mobilenet.enf) as sess: | ||
mobilenet = SSDMobileNet(postprocessor_type="Rust") |
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.
postprocessor_type
의 타입을 보니 Union[str, Platform]
로 되어있는데요. 값을 str
타입으로 쓰고있는 이유가 무엇일까요? Platform
을 쓰는 것이 좋을 것 같은데, 특별한 이유가 있었을 것 같기도 해서요.
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.
Platform 이 strEnum 이기도 하고, 사용자가 편하게 문자열로 선택하면 좋을 것 같아서 그렇게 했습니다. 사실 strict 하게 typing 하자면 Platform만 주는 것이 맞긴 한데요, 모델 주는 사용자 편의성이 중요한 프로젝트인 것 같아서 임의로 그렇게 했습니다. 아님 혹시 더 좋은 방법이 있을까요?
tests/bench/test_ssd_mobilenet.py
Outdated
@@ -61,9 +61,9 @@ def workload(image_id, image): | |||
} | |||
detections.append(detection) | |||
|
|||
sess = session.create(model.enf) | |||
runner = create_runner(model.model_source()) |
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.
다른 테스트들과는 달리 with
로 runner를 생성하지 않고 있는데, 이유가 있는 것일까요?
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.
dc0aecc 에서 반영했습니다.
affinity: | ||
nodeAffinity: | ||
requiredDuringSchedulingIgnoredDuringExecution: | ||
nodeSelectorTerms: | ||
- matchExpressions: | ||
- key: role | ||
operator: In | ||
values: | ||
- npu |
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.
affinity는 생략해도 될 것 같습니다. (resources의 npu 지정에 의해 충족됨)
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.
dc0aecc 에서 반영했습니다.
nodeAffinity: | ||
requiredDuringSchedulingIgnoredDuringExecution: | ||
nodeSelectorTerms: | ||
- matchExpressions: | ||
- key: role | ||
operator: In | ||
values: | ||
- npu |
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.
affinity는 생략해도 될 것 같습니다. (resources의 npu 지정에 의해 충족됨)
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.
dc0aecc 에서 반영했습니다.
furiosa/models/vision/yolov5/core.py
Outdated
@@ -97,13 +105,13 @@ def _reshape_output(feat: np.ndarray, anchor_per_layer_count: int, num_classes: | |||
class YOLOv5PreProcessor(PreProcessor): | |||
@staticmethod | |||
def __call__( | |||
images: Sequence[Union[str, np.ndarray]], with_quantize: bool = False | |||
images: Sequence[Union[str, np.ndarray]], skip_quantize: bool = True | |||
) -> Tuple[np.ndarray, List[Dict[str, Any]]]: | |||
"""Preprocess input images to a batch of input tensors | |||
|
|||
Args: | |||
images: Color images have (NCHW: Batch, Channel, Height, Width) dimensions. |
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과는 관계가 없습니다만, input image는 NHWC가 아닌지 궁금합니다.
아래의 transpose([2, 0, 1])
이 NHWC -> NCHW 를 하는게 아닌가 해서요.
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.
cad487e 에서 적용했습니다~
docs/examples/ssd_mobilenet_onnx.py
Outdated
inputs, contexts = mobilenet.preprocess(image) | ||
outputs = sess.run(inputs).numpy() | ||
with create_runner(quantized_onnx, compiler_config=compiler_config) as runner: | ||
inputs, contexts = mobilenet.preprocess(image, skip_quantize=False) |
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.
이 커멘트는 가볍게 드리는 의견입니다. 다른 분들도 견해를 들려주시면 좋을 것 같습니다.
기존의 with_quantize
나 이번의 skip_quantize
라는 이름을 보았을 때,
quantize를 왜 해야하는지? 언제 skip 하는지? 어떤 효과를 주는지? 마음대로 설정할 수 있는지? 에 대한 의문이 생깁니다.
context를 아는 경우에는 조금만 생각해보면 알 수 있지만, 아닌 일반 사용자는 쉽게 이해할 수 있을지 모르겠네요.
저는 모델의 입력 텐서 타입이 float인지 int(uint)인지가 중요하지 않을까, 하고 생각이 들었는데요,
float 라면 image를 read한 값을 형변환을 할 것이고, uint라면 변환을 생략할 것이기 때문입니다.
결과적으로 quantize 여부는 타입에 의존적이게 되므로, 이를 인자로 True/False로 지정하여 선택이 가능한 것 처럼 보이기보다는
모델이 어떤 타입인지를 표현하는 인자가 되면 보다 설명이 간단해지지 않을까 싶습니다.
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.
cad487e 에서 with_scaling이라는 파라미터로 바꾸었고 설명을 추가했습니다. 구두로 논의 나눈 내용이라 자세히 적지는 않겠습니다. 감사합니다
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 이 크기 때문에 이 관련해서는 별도 PR 로 다루면 좋을 것 같기는 합니다만, 조금 설명을 듣고 싶은데요. with_scaling 이라는 이름이 여전히 모호한 느낌이 있는데요. 제가 인용한 병찬님이 말씀 처럼 use_fp32 = True
같은 옵션이나 dtype을 직접 넣는 것을 고려하지 않았는지 궁금하기도 합니다.
@hyunsik 머지를 위한 최종 리뷰 부탁드리겠습니다. |
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.
고생 많으셨습니다. with_scaling
옵션에 대한 의견 코멘트로 달았습니다. 이 관련 논의와 변경은 필요한 경우에 별도 이슈나 PR 에서 해도 좋을 것 같습니다. 바로 머지하셔도 좋을 것 같습니다.
a. 대신, 해당 필드에 접근할 때 lazy load 합니다.
b. 한 번에 모든 바이너리를 다운로드 받는 resolve_all 함수가 추가되었습니다.
Model.load()
,Model.load_async()
같은 함수를 사용하지 않습니다.a. 직관적으로
Model()
을 불러 모델 클래스를 만들 수 있습니다.b. 어떤 postprocess를 사용할지는 기존과 비슷하게
Model(postprocessor_type="RUST")
를 이용해 정할 수 있습니다.c. 이제 파일을 로컬에 캐시하므로, Model load가 오래 걸리는 것은 컴퓨터당 한 번 정도일 것인데 이걸 최적화하겠다고 async initializer를 쓸 이유가 없다고 판단
d. 네트워크 환경이 좋지 않은 곳에서 실행하는 경우가 많은데, 이 때 어차피 network throughput에 block되므로 concurrent 하게 바이너리를 다운로드 받을 이유가 없음
a. 알맞은 postprocessor 를 사용해 init 했는지 validation하는 과정이 추가되었습니다.(지원하는 postprocess목록에 있는지)
furiosa-models list
나furiosa-models desc
같은 명령이 빨라졌습니다.a. Lazy loading 덕분입니다.
a. 원본 f32 onnx|tflite 필드
source
->origin
- 이유: furiosa rt에서 model source라는 이름으로 Runner의 입력이 될 수 있는 것들을 총칭하기 때문에 헷갈릴 여지가 있다고 판단
b. Calibration range 필드
calib_yaml
->tensor_name_to_range
- 일단, yaml 대신 Python dict를 돌려주도록 변경
- furiosa-quantizer 의 이름에 맞춤
c. ENF 필드
enf
,enf_1pe
->model_source()
,model_source(num_pe=1)
- ENF라는 이름을 사용자에게 노출할 이유가 없음
- 함수로 바꿔서 pe 수를 조정 가능하게 함, 추후엔 remove_lower 등 더 다양한 옵션을 지원하게 하려고 함