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

[DOC] Update Model Links to Include Commit #17015

Merged
merged 2 commits into from
Jun 6, 2024
Merged

Conversation

jhong92-pro
Copy link
Contributor

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:
onnx/models#644

@leandron leandron self-assigned this May 22, 2024
Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. There is a manual step to cache this file in S3. I will do it and report back when it's done.

Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

OK, the manual step should be complete now.

Can you add a new line to this file:

"https://github.com/onnx/models/raw/main/vision/classification/resnet/model/resnet50-v2-7.onnx": f"{BASE}/2022-10-05/resnet50-v2-7.onnx",

The first part is the URL you just modified in the PR, and the second should be f"{BASE}/2024-05-22/resnet50-v2-7.onnx".

This should unblock it in CI.

For clarity, here is the manual step that we cached the file: https://github.com/apache/tvm/actions/runs/9188515282

Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @jhong92-pro

@jhong92-pro
Copy link
Contributor Author

@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?

"https://github.com/onnx/models/raw/bd206494e8b6a27b25e5cf7199dbcdbfe9d05d1c/vision/classification/mnist/model/mnist-1.onnx": f"{BASE}/onnx/mnist-1.onnx",
"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",

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

@lhutton1
Copy link
Contributor

@tvm-bot rerun

@lhutton1
Copy link
Contributor

lhutton1 commented May 30, 2024

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

@jhong92-pro
Copy link
Contributor Author

jhong92-pro commented May 30, 2024

@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.

@lhutton1
Copy link
Contributor

Happy to do whichever works best for you :)

@lhutton1
Copy link
Contributor

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

@jhong92-pro
Copy link
Contributor Author

@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/onnx/models/raw/main/vision/classification/mobilenet/model/mobilenetv2-7.onnx": f"{BASE}/onnx/models/raw/main/vision/classification/mobilenet/model/mobilenetv2-7.onnx",
"https://github.com/onnx/models/raw/main/vision/classification/resnet/model/resnet50-v2-7.onnx": f"{BASE}/2022-10-05/resnet50-v2-7.onnx",

@lhutton1
Copy link
Contributor

No problem, I think we can just rewrite them with the updated paths (both the key and value of the URL_MAP)

@jhong92-pro jhong92-pro changed the title [autoTVM] Update link for onnx resnet in documentation [DOC] Update Model Links to Include Commit May 30, 2024
@lhutton1
Copy link
Contributor

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", 

@jhong92-pro
Copy link
Contributor Author

@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?

@lhutton1
Copy link
Contributor

lhutton1 commented Jun 3, 2024

Hi @jhong92-pro, if you would be able to add it here, that would be great!

@lhutton1
Copy link
Contributor

lhutton1 commented Jun 3, 2024

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.

@jhong92-pro
Copy link
Contributor Author

jhong92-pro commented Jun 6, 2024

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

@lhutton1
Copy link
Contributor

lhutton1 commented Jun 6, 2024

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"

@lhutton1 lhutton1 merged commit 1f4c568 into apache:main Jun 6, 2024
18 checks passed
@lhutton1
Copy link
Contributor

lhutton1 commented Jun 6, 2024

Thanks for the continued effort on this @jhong92-pro! And thanks for the reviews @leandron

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 this pull request may close these issues.

None yet

3 participants