-
Notifications
You must be signed in to change notification settings - Fork 114
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
- made NDEBUG on in release #3427
base: master
Are you sure you want to change the base?
Conversation
hnil
commented
Mar 6, 2023
- make NDEBUG on in release.
- draft pullrequest to test performance on ytelse
jenkins build this please |
benchmark please |
Benchmark result overview:
View result details @ https://www.ytelses.com/opm/?page=result&id=1986 |
Benchmark result overview:
View result details @ https://www.ytelses.com/opm/?page=result&id=1986 |
This is really strange on spe9 the effect i > 25 % on update on my machine single threaded. |
I think the time has come for this to be merged. The only reason to NOT define NDEBUG for release builds is to preserve assert()s in the executable as a last-resort guard against producing wrong results. Such assert()s have now been removed from most parts of the code (although a few remain) and replaced with The remaining assert()s combined with not defining NDEBUG can result in users experiencing errors terminating their runs, when it in some cases at least could have been possible to run through after timestep cuts. That is another reason to make this change. |