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

Reshape offset_re_shape has incorrect value #261

Closed
lutzroeder opened this issue Apr 14, 2020 · 5 comments
Closed

Reshape offset_re_shape has incorrect value #261

lutzroeder opened this issue Apr 14, 2020 · 5 comments
Labels
bug

Comments

@lutzroeder
Copy link

@lutzroeder lutzroeder commented Apr 14, 2020

Model zoo file detect_tflite.tmfile contains a Reshape operator at address 23244.

offset_t_param = 23220 points to this TM2_ReshapeParam data:

01 00 00 00 06 00 00 00 5B 00 00 00

0x0000005B is not a valid pointer for offset_re_shape?

04b6791 seems to have changed the serialization format without updating the version number?

@pierricklee @LEMON-LM @@BUG1989

lutzroeder added a commit to lutzroeder/netron that referenced this issue Apr 14, 2020
@pierricklee
Copy link
Collaborator

@pierricklee pierricklee commented Apr 14, 2020

Hi, Lutz,

Thanks for supporting Tengine!
Actaully, this tmfile model on google drive is out of date compare to the latest version of Tengine v1.12.
The data here you found is aligned by

>     int32_t dim_0;
>     int32_t dim_1;
>     int32_t dim_2;
>     int32_t dim_3;
>     int32_t dim_size;
>     int32_t axis;`

04b6791 has changed the TM2_ReshapeParam here:
image

The model on google drive used the previous format. So it makes sense with 0x0001 * 0x0006 * 0x005B, which equals 1 * 6 * 91.


With the latest model:
image

You could use this model: https://github.com/pierricklee/tmfile-sample/blob/master/detect_tflite.tmfile

@LEMON-LM Please update this model to google drive.

@lutzroeder
Copy link
Author

@lutzroeder lutzroeder commented Apr 14, 2020

@pierricklee Users will have copies of the old file and the same happens for other files. Users might use older versions of Tengine and might still open older files in Netron which is now going to crash when the old format is loaded. You can't call it the old format because it looks identical to the new format and tools can't tell the difference.

This is not acceptable. If you change the format and want things to continue working you need to update version numbers (either the version for the operator or the version number for the entire model) so tools can support both versions. Tools can't be crashing because there is no way to handle this.

@pierricklee
Copy link
Collaborator

@pierricklee pierricklee commented Apr 15, 2020

@lutzroeder That is a good suggestion! We are planning add an opset version and magic number into tmfile model. This will be on Tengine roadmap as soon as possible. I will keep you updated.

@BUG1989 BUG1989 added the bug label Apr 15, 2020
@lutzroeder
Copy link
Author

@lutzroeder lutzroeder commented Apr 15, 2020

@pierricklee op_ver exists on TM2_Operator already (there is also model version). It needs to be updated for breaking changes like this and all new files need to be regenerated to include the new version number. The longer you wait, the more these incompatible identical looking files will spread causing problems.

@pierricklee
Copy link
Collaborator

@pierricklee pierricklee commented Apr 15, 2020

@lutzroeder That's the point. We are rewriting the op_ver part and will regenerate model files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.