-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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 for quantize fail on init due to undefined static initialization of complex objects #927
Conversation
…on on first use. This prevents an undefined behavior on program run, for example, crash in Release build, works in Debug build
@arikpoz this MR still can not fix the issue. |
Hmm, how is this UB - on which compiler / platform? |
I was able to reproduce it and fix it on Windows 10 with MSVC compiler, where the exception came from the static initialization of the maps. It could be the original reported issue was a different exception |
@jayliang701 , do you get the same behavior if you compile in Debug vs. Release? |
still using Macbook Air M1 (2020), Memory 16G yes. I have tried |
@ggerganov , after fixed AVX flag mentioned in PR #809 , the original crash on static initialization I had with quantize disappeared. That said, I believe this is random behavior of the compiler and we should keep the changes suggested in this PR, since when you have static initialization of complex objects that uses other modules (std::) there is no guaranteed order for running the static initializers across the different modules, so the behavior is undefined. See https://en.cppreference.com/w/cpp/language/siof for reference |
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 really don't think these static maps can ever cause initialization order problems (or any other problems) as they are used only in the current translation unit.
But anyway, it does not hurt to change it as suggested.
Just make the functions static and return const references
Updated code as requested |
@ggerganov , let me know if something is missing to merge this |
* Add MistralLite format * Update llama_chat_format.py * Update llama_chat_format.py
Addresses issue #920
Replaced static initialization of complex objects with a initialization on first use. This prevents an undefined behavior on program run, for example, crash in Release build, works in Debug build