-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[BugFix][MSC] split name_string with index by colon from the right #17000
Conversation
Fixes a naming mismatch in MSCGraph where tensor_name could formatted as 'string:index:index',and the corresponding node.name is 'string:index'. Splitting tensor_name from the right aligns it correctly. For example, the TFLite default input name 'serving_default_input:0' becomes 'serving_default_input:0:0' in MSCGraph, while node.name remains 'serving_default_input:0'.
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.
Thanks for the fix @psunn, would it be possible to add a test case?
cc @Archermmt |
Hi @Archermmt, could you help to review this PR? thanks in advance. |
Seem ok for me. The only change in this PR is the split direction, which has no influence on current test. I think the main reason is that: in some corner cases like TFLite model, the node name looks like "NODE:IDX", which is normally the tensor name format. |
@Archermmt thanks for review. I force-pushed only to address linting issues. @lhutton1 @Archermmt @Hzfengsy |
Thanks @psunn @Archermmt @Hzfengsy! |
Fixes a naming mismatch in MSCGraph where tensor_name could formatted as 'string:index:index',and the corresponding node.name is 'string:index'. Splitting tensor_name from the right aligns it correctly.
For example, the TFLite default input name 'serving_default_input:0' becomes 'serving_default_input:0:0' in MSCGraph, while node.name remains 'serving_default_input:0'.