-
Notifications
You must be signed in to change notification settings - Fork 866
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
Issue 1064 #1140
Issue 1064 #1140
Conversation
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.
@@ -32,7 +32,7 @@ | |||
"tolerance":5 | |||
}, | |||
{ | |||
"url":"https://torchserve.pytorch.org/mar_files/squeezenet1_1.mar", | |||
"url":"{{mar_path_squeezenet1_1}}", |
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.
@lxning seems the PR handles the mar creation for eager mode only at this stage?
Is the reason for the text_classification not being auto generated related to training step required prior to archiving the model?
This seems to be the same case with Bert mar file as well?
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.
text_classification has issue with PT1.9 so it is not handled at here.
@@ -2,7 +2,7 @@ Use following properties to add inference test case in inference_data.json | |||
|
|||
Mandatory properties | |||
---- | |||
**url:** Model url. | |||
**url:** Model url or a varialbe {{mar_path_xxxx}} defined in environment.json. xxxx is replaced with model name. This model's mar file is generated by ts_scripts/marsgen.py via ts_scripts/mar_config.json. |
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.
@lxning it would be helpful to add an example for {{mar_path_xxxx}}, is the first part getting used to recognize the variable "mar_path"?
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.
mar_path is the key used for identifying local mar or remote url. "mar_path_xxxx" is used in environment.json. Pls check.
@@ -104,7 +102,10 @@ def test_sanity(): | |||
model_handler = model["handler"] | |||
|
|||
# Run gRPC sanity | |||
register_model_grpc_cmd = f"python ts_scripts/torchserve_grpc_client.py register {model_name}" | |||
print("pass mg.mar_set=", mg.mar_set) |
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.
Nit: this perhaps needs to be cleaned up
if mar_set_str: | ||
mar_set = set(mar_set_str.split(',')) | ||
marfile = f"{model_name}.mar" | ||
print(f"## Check {marfile} in mar_set :", mar_set) |
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.
Nit: can be cleaned up
Description
Please include a summary of the feature or issue being fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes #(issue)
#1064
Type of change
Please delete options that are not relevant.
Feature/Issue validation/testing
Please describe the tests [UT/IT] that you ran to verify your changes and relevent result summary. Provide instructions so it can be reproduced.
Please also list any relevant details for your test configuration.
Test A: sanity test
Test B: regression test
UT/IT execution results
Logs
regression test result
Checklist:
Code changes description: