Closed jhong92-pro closed 4 months ago
@leandron,
I reviewed the links in request_hook.py
and found that other links include the commit hash in the URL to ensure they remain unchanged. I think it would be beneficial to update the links to include the commit hash to prevent future changes, rather than changing to a new URL. What do you think?
grep -r "resnet50-v2-7\.onnx" *
apps/wasm-standalone/wasm-graph/tools/build_graph_lib.py: "resnet50-v2-7.onnx"
apps/wasm-standalone/wasm-graph/tools/build_graph_lib.py: model_path = download_testdata(model_url, "resnet50-v2-7.onnx", module="onnx")
gallery/tutorial/tvmc_command_line_driver.py:# wget https://github.com/onnx/models/raw/b9a54e89508f101a1611cd64f4ef56b9cb62c7cf/vision/classification/resnet/model/resnet50-v2-7.onnx
gallery/tutorial/tvmc_command_line_driver.py:# resnet50-v2-7.onnx
gallery/tutorial/tvmc_command_line_driver.py:# resnet50-v2-7.onnx
gallery/tutorial/tvmc_command_line_driver.py:# resnet50-v2-7.onnx
gallery/tutorial/tvmc_command_line_driver.py:# resnet50-v2-7.onnx
gallery/tutorial/autotvm_relay_x86.py: "resnet50-v2-7.onnx"
gallery/tutorial/autotvm_relay_x86.py:model_path = download_testdata(model_url, "resnet50-v2-7.onnx", module="onnx")
gallery/tutorial/tvmc_python.py: wget https://github.com/onnx/models/raw/b9a54e89508f101a1611cd64f4ef56b9cb62c7cf/vision/classification/resnet/model/resnet50-v2-7.onnx
gallery/tutorial/tvmc_python.py: mv resnet50-v2-7.onnx my_model.onnx
tests/python/driver/tvmc/conftest.py: file_to_download = "resnet50-v2-7.onnx"
tests/python/contrib/test_bnns/test_onnx_topologies.py: "ResNet50-v2": "vision/classification/resnet/model/resnet50-v2-7.onnx",
tests/python/relay/collage/menangerie.py: "filename": "resnet50-v2-7.onnx",
tests/scripts/request_hook/request_hook.py: "https://github.com/onnx/models/raw/bd206494e8b6a27b25e5cf7199dbcdbfe9d05d1c/vision/classification/resnet/model/resnet50-v2-7.onnx": f"{BASE}/onnx/models/raw/bd206494e8b6a27b25e5cf7199dbcdbfe9d05d1c/vision/classification/resnet/model/resnet50-v2-7.onnx",
tests/scripts/request_hook/request_hook.py: "https://github.com/onnx/models/raw/main/vision/classification/resnet/model/resnet50-v2-7.onnx": f"{BASE}/2022-10-05/resnet50-v2-7.onnx",
grep -r "mobilenetv2-7\.onnx" *
tests/python/contrib/test_hexagon/test_models.py: model_url = "https://github.com/onnx/models/raw/main/vision/classification/mobilenet/model/mobilenetv2-7.onnx" # pylint: disable=line-too-long
tests/python/contrib/test_hexagon/test_models.py: model_url, "mobilenetv2-7.onnx", module="onnx"
tests/python/contrib/test_hexagon/test_relax_integration.py: model_url = "https://github.com/onnx/models/raw/main/vision/classification/mobilenet/model/mobilenetv2-7.onnx"
tests/python/contrib/test_hexagon/test_relax_integration.py: model_url, "mobilenetv2-7.onnx", module="onnx"
tests/python/contrib/test_bnns/test_onnx_topologies.py: "MobileNet-v2": "vision/classification/mobilenet/model/mobilenetv2-7.onnx",
tests/scripts/request_hook/request_hook.py: "https://github.com/onnx/models/raw/main/vision/classification/mobilenet/model/mobilenetv2-7.onnx": f"{BASE}/onnx/models/raw/main/vision/classification/mobilenet/model/mobilenetv2-7.onnx",
For example,
https://github.com/onnx/models/raw/main/vision/classification/mobilenet/model/mobilenetv2-7.onnx
to https://github.com/onnx/models/raw/bd206494e8b6a27b25e5cf7199dbcdbfe9d05d1c/vision/classification/mobilenet/model/mobilenetv2-7.onnx
and
https://github.com/onnx/models/raw/main/vision/classification/resnet/model/resnet50-v2-7.onnx
to
https://github.com/onnx/models/raw/bd206494e8b6a27b25e5cf7199dbcdbfe9d05d1c/vision/classification/resnet/model/resnet50-v2-7.onnx
@tvm-bot rerun
I reviewed the links in
request_hook.py
and found that other links include the commit hash in the URL to ensure they remain unchanged. I think it would be beneficial to update the links to include the commit hash to prevent future changes, rather than changing to a new URL. What do you think?
~In general this is a good idea, however, since the links redirect to an artefact uploaded to https://tvm-ci-resources.s3.us-west-2.amazonaws.com, I don't think there would be much benefit in this instance~ Apologies, I was thinking of only the CI case, yes point the link to a specific commit is a good idea
@lhutton1
Thank you for your answer. I will update the links for resnet.onnx
and mobilenet.onnx
to include the commit hash. I will force push updates to the patch-1 branch to clean up the commit history and ensure the links point to the specific commit. Please let me know if this is acceptable, or if you prefer, I can create a new PR.
Happy to do whichever works best for you :)
Thanks for the update, quick reminder that we'd still need to add these new links to request_hook.py. The manual step that @leandron mentioned previously will also need completing again
@lhutton1
Thank you!
Additionally, the original links will no longer be used. Please let me know if I can delete or comment them out. https://github.com/apache/tvm/blob/7c2c0d9337f3b353576bccc30f61c16abcc633a7/tests/scripts/request_hook/request_hook.py#L124-L125
No problem, I think we can just rewrite them with the updated paths (both the key and value of the URL_MAP)
I've just completed the manual step for mobilenetv2-7.onnx. It seems resnet-v2-7.onnx from the same commit hash has already been uploaded here.
We'd just need to add a new line to request_hooks.py:
"https://github.com/onnx/models/raw/bd206494e8b6a27b25e5cf7199dbcdbfe9d05d1c/vision/classification/mobilenet/model/mobilenetv2-7.onnx": f"{BASE}/m/onnx/models/raw/bd206494e8b6a27b25e5cf7199dbcdbfe9d05d1c/vision/classification/mobilenet/model/mobilenetv2-7.onnx",
@lhutton1
Just to confirm, would you like me to go ahead and rewrite the paths with the updated ones in the URL_MAP, or will you be handling that?
Hi @jhong92-pro, if you would be able to add it here, that would be great!
Many apologies, there was a mistake in the line I shared above, somehow I managed to add an additional /m/
to the redirect path. Changing the line to:
"https://github.com/onnx/models/raw/bd206494e8b6a27b25e5cf7199dbcdbfe9d05d1c/vision/classification/mobilenet/model/mobilenetv2-7.onnx": f"{BASE}/onnx/models/raw/bd206494e8b6a27b25e5cf7199dbcdbfe9d05d1c/vision/classification/mobilenet/model/mobilenetv2-7.onnx",
should fix it.
I apologize for my misunderstanding. I encountered an issue related to the input names of the mobilenetv2-7.onnx model. After some investigation, I found that the input name was changed from input
to data
in this PR
EDIT
I have updated the commit hash for mobilenetv2-7 :
https://github.com/onnx/models/raw/131c99da401c757207a40189385410e238ed0934/vision/classification/mobilenet/model/mobilenetv2-7.onnx
Hi @jhong92-pro, I've completed the manual step, this line would need adding to request_hook.py again:
"https://github.com/onnx/models/raw/131c99da401c757207a40189385410e238ed0934/vision/classification/mobilenet/model/mobilenetv2-7.onnx": f"{BASE}/onnx/models/raw/131c99da401c757207a40189385410e238ed0934/vision/classification/mobilenet/model/mobilenetv2-7.onnx"
Thanks for the continued effort on this @jhong92-pro! And thanks for the reviews @leandron
The ONNX pretrained ResNet model URLs have been updated in the autoTVM documentation. The previous URLs are no longer valid, and this change points to the correct URLs.
Related PR: https://github.com/onnx/models/pull/644