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

A lot of ifs can be replaced by if constexpr #3163

Open
jachymb opened this issue Mar 11, 2025 · 3 comments
Open

A lot of ifs can be replaced by if constexpr #3163

jachymb opened this issue Mar 11, 2025 · 3 comments

Comments

@jachymb
Copy link

jachymb commented Mar 11, 2025

This is based on review by @SteveBronder in my PR #3157 (comment) but it apparently pertains to many other already implemented distributions as well as functions

When I run the following command

grep -rE if \(!?is_constant stan/math

The output has 1146 lines.

If I understand correctly the value is known at compile time and these could be replaced by if constexpr

similarly:

grep -rE if \(!?include_summand stan/math

has 285 lines.

This is just for illustration, there are others that can be found that can be found by filtering for <.*> or ::value etc. The grep may not find all in case of formatting line breaks. Perhaps this can be done by a more sophisticated regex search and replace through the codebase?

@SteveBronder
Copy link
Collaborator

Yes! I have not had time, but what I would really like to do is replace !is_constant and !is_all_constant with a helper variable template like is_autodiff_type_v. Then use constexpr everywhere we have is_constant. So lines like

if (!is_constant_all<T_beta, T_x, T_alpha>::value) {

would be

if constexpr (is_all_autodiff_type_v<T_beta, T_x, T_alpha>)

The helper types would look like

/**
 * Checks whether the `scalar_type` of a type is an autodiff type
 * @tparam T Any type
 */
template <typename T>
struct is_autodiff_type : is_autodiff<scalar_type_t<T>> {};

template <typename T>
inline constexpr bool is_autodiff_type_v = is_autodiff_type<T>::value;

template <typename... Types>
inline constexpr bool is_all_autodiff_type_v = std::conjunction<is_autodiff_type<Types>...>::value;

@spinkney
Copy link
Collaborator

Yes! I have not had time, but what I would really like to do is replace !is_constant and !is_all_constant with a helper variable template like is_autodiff_type_v. Then use constexpr everywhere we have is_constant. So lines like

if (!is_constant_all<T_beta, T_x, T_alpha>::value) {

would be

if constexpr (is_all_autodiff_type_v<T_beta, T_x, T_alpha>)

The helper types would look like

/**
 * Checks whether the `scalar_type` of a type is an autodiff type
 * @tparam T Any type
 */
template <typename T>
struct is_autodiff_type : is_autodiff<scalar_type_t<T>> {};

template <typename T>
inline constexpr bool is_autodiff_type_v = is_autodiff_type<T>::value;

template <typename... Types>
inline constexpr bool is_all_autodiff_type_v = std::conjunction<is_autodiff_type<Types>...>::value;

Does having this template make it easier to get user declared derivatives and marking parameters as constants, ie don't differentiate? or par variable syntax which possible

@SteveBronder
Copy link
Collaborator

It's mostly to prettify the code a bit. It would allow you to do different things for scalars and vectors which can be nice, but otherwise everything should be the same

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

No branches or pull requests

3 participants