set static const values to static constexpr#2830
Conversation
…imization still treats them as known as compile time
…to fix/constexpr-constants
…to fix/constexpr-constants
|
@SteveBronder is this ready for review? |
|
|
||
| // \cond | ||
| static const std::string add_batch_kernel_code = STRINGIFY( | ||
| static constexpr const char *add_batch_kernel_code = STRINGIFY( |
There was a problem hiding this comment.
| static constexpr const char *add_batch_kernel_code = STRINGIFY( | |
| static constexpr const char* add_batch_kernel_code = STRINGIFY( |
For consistency
| namespace opencl_kernels { | ||
| // \cond | ||
| static const std::string is_symmetric_kernel_code = STRINGIFY( | ||
| static constexpr const char *is_symmetric_kernel_code = STRINGIFY( |
There was a problem hiding this comment.
| static constexpr const char *is_symmetric_kernel_code = STRINGIFY( | |
| static constexpr const char* is_symmetric_kernel_code = STRINGIFY( |
|
|
||
| // \cond | ||
| static const char *cumulative_sum1_kernel_code = STRINGIFY( | ||
| static constexpr const char *cumulative_sum1_kernel_code = STRINGIFY( |
There was a problem hiding this comment.
| static constexpr const char *cumulative_sum1_kernel_code = STRINGIFY( | |
| static constexpr const char* cumulative_sum1_kernel_code = STRINGIFY( |
|
|
||
| // \cond | ||
| static const char *cumulative_sum2_kernel_code = STRINGIFY( | ||
| static constexpr const char *cumulative_sum2_kernel_code = STRINGIFY( |
There was a problem hiding this comment.
| static constexpr const char *cumulative_sum2_kernel_code = STRINGIFY( | |
| static constexpr const char* cumulative_sum2_kernel_code = STRINGIFY( |
stan/math/prim/fun/constants.hpp
Outdated
| static constexpr double LOG_PI | ||
| = 1.14472988584940017414342735135305871164729481291531157151362; |
There was a problem hiding this comment.
This is a bit of a departure from how we've stored these constants so far, and not something I'm a huge fan of tbh, is it an essential change here?
Not a blocking comment if there's no alternative, just personal preference
There was a problem hiding this comment.
Actually! We can do this directly using boost's constants:
static constexpr double LOG_PI = 2 * boost::math::constants::log_root_two_pi<double>() - LOG_TWO;
stan/math/prim/fun/log.hpp
Outdated
| static constexpr double inf = std::numeric_limits<double>::infinity(); | ||
| static constexpr double nan = std::numeric_limits<double>::quiet_NaN(); |
There was a problem hiding this comment.
This header should just use the NOT_A_NUMBER and INFTY constants directly
stan/math/prim/fun/log10.hpp
Outdated
| template <typename V> | ||
| inline std::complex<V> complex_log10(const std::complex<V>& z) { | ||
| static const double inv_log_10 = 1 / std::log(10); | ||
| static constexpr double inv_log_10 = 1 / 2.30258509299404568401799145468; |
There was a problem hiding this comment.
| static constexpr double inv_log_10 = 1 / 2.30258509299404568401799145468; | |
| static constexpr double inv_log_10 = 1 / LOG_TEN; |
stan/math/prim/prob/wiener_lpdf.hpp
Outdated
| static constexpr double LOG_PI_LOG_WIENER_ERR | ||
| = LOG_PI + -13.81551055796427410410794872810618524; |
There was a problem hiding this comment.
| static constexpr double LOG_PI_LOG_WIENER_ERR | |
| = LOG_PI + -13.81551055796427410410794872810618524; | |
| static constexpr double LOG_PI_LOG_WIENER_ERR | |
| = LOG_PI - 6 * LOG_TEN; |
stan/math/prim/prob/wiener_lpdf.hpp
Outdated
| static constexpr double SQUARE_PI_OVER_TWO | ||
| = 9.869604401089358618834490999876151135 * 0.5; |
There was a problem hiding this comment.
| static constexpr double SQUARE_PI_OVER_TWO | |
| = 9.869604401089358618834490999876151135 * 0.5; | |
| static constexpr double SQUARE_PI_OVER_TWO = (pi() * pi()) / 2; |
stan/math/prim/fun/constants.hpp
Outdated
| static constexpr double LOG_SQRT_PI | ||
| = 0.5723649429247000870717136756765293558236474064576557857568115357; |
There was a problem hiding this comment.
| static constexpr double LOG_SQRT_PI | |
| = 0.5723649429247000870717136756765293558236474064576557857568115357; | |
| static constexpr double LOG_SQRT_PI = LOG_PI / 2; |
One more simplification I thought of
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
|
@andrjohns thank you for taking this over the finish line! |
Summary
This is to fix a bug where some static initialization fiascos can happen when compiling at
-O0. I went through and set many of the values fromstatic consttostatic constexpr. This should only affect things when compiling at-O0since all the other optimizations levels treatstatic constvalues as known at compile time.Tests
No new tests
Side Effects
Nope. Though I'm away from my desktop atm so these might fail the OpenCL tests as I was not able to test them locally.
Release notes
replaces
static constobjects withstatic constexprwhen available.Checklist
Math issue #(issue number)
Copyright holder: Steve Bronder
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit)make test-headers)make test-math-dependencies)make doxygen)make cpplint)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested