-
Notifications
You must be signed in to change notification settings - Fork 170
[tools/onnx-subgraph] add functions for json config process #15152
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
Conversation
add functions for json config process ONE-DCO-1.0-Signed-off-by: Youxin Chen <yx113.chen@samsung.com>
| std::ifstream in(json_path, std::ios::binary); | ||
| if (!in.is_open()) | ||
| { | ||
| std::cout << "Error opening file\n"; |
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.
below code used this
| std::cout << "Error opening file\n"; | |
| std::cerr << "Error opening file." << std::endl; |
maybe use single convention
| if (reader.parse(in, root)) | ||
| { | ||
| // Extract and set the maximum subgraph size from hardware limits | ||
| float max_subgraph_size_json = root["hardware_limits"]["max_subgraph_size"].asFloat(); |
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.
Q) what if file has wrong content and there is no such ["hardware_limits"]["max_subgraph_size"] ?
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.
yes, it is a problem that I don't consider such exception cases in this PR, current I suppose there are such contents in the config.json, I will add it in next PR, thank you!
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.
I have tested, if there was no such ["hardware_limits"]["max_subgraph_size"], max_subgraph_size_json =0, and no exception happened, thank you
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.
OK, thanks for checking.
Without such nodes, having max_subgraph_size_json = 0, can this be correct interpretation of the json file?
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.
for safety using, need to check the existing by using if(root.contains("hardware_limits")), I will add it in future code
seanshpark
left a comment
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.
LGTM
related issue: #14534
historical full changes: #14613
add functions for json config process
ONE-DCO-1.0-Signed-off-by: Youxin Chen yx113.chen@samsung.com