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

Fix FLT_MAX ONNX -> NCNN error #2684

Merged
merged 1 commit into from
Mar 21, 2024
Merged

Fix FLT_MAX ONNX -> NCNN error #2684

merged 1 commit into from
Mar 21, 2024

Conversation

joeyballentine
Copy link
Member

Finally looked into this bug. Basically, there's code that checks if a default value is a string and does some stuff if it is. Since FLT_MAX isn't a number and is instead a string, it was triggering the code to handle strings, when it really should be handled like a number. So, I just added a case to the if statement to ignore FLT_MAX and -FLT_MAX

@joeyballentine joeyballentine marked this pull request as draft March 18, 2024 21:32
@joeyballentine
Copy link
Member Author

Converting this to a draft until i can figure out whats happening with the resulting NCNN models crashing the backend

@joeyballentine joeyballentine marked this pull request as ready for review March 18, 2024 21:49
@joeyballentine
Copy link
Member Author

The model converts fine. The issue seems unrelated. Perhaps SPAN just can't be run in ncnn?

@Kim2091
Copy link
Collaborator

Kim2091 commented Mar 19, 2024

@joeyballentine If you take a model converted to ONNX in new chaiNNer, and try to convert it to NCNN in an older version of chaiNNer this error happens no matter what arch it is.

@joeyballentine
Copy link
Member Author

@Kim2091 that's unrelated. The float max error is due to the clipping introduced via spandrel with the call API. The crash I believe is arch dependent. This PR merely addresses the float max problem

@joeyballentine joeyballentine merged commit 5187860 into main Mar 21, 2024
14 checks passed
@joeyballentine joeyballentine deleted the fix-flt_max branch March 21, 2024 14:06
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.

2 participants