| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
Can this use apply_vector_unary? That way the Eigen implementation will also be used for std::vector (and nested containers) |
Sorry, something went wrong.
|
@andrjohns do you mean something besides the definition in prim that uses apply_scalar_unary? template <typename T, require_std_vector_t<T>* = nullptr>
inline auto inv_logit(T&& x) {
return apply_scalar_unary<inv_logit_fun, std::decay_t<T>>::apply(
std::forward<T>(x));
} |
Sorry, something went wrong.
Yeah the issue with the current definition is that it won't use the vectorised Eigen implementation for std::vector types (either std::vector<double> or std::vector<Eigen::...>). You can instead use the pattern from prim/fun/exp where apply_scalar_unary is used for non-arithmetic containers and apply_vector_unary is used for arithmetic containers: template <typename Container, require_ad_container_t<Container>* = nullptr>
inline auto inv_logit(const Container& x) {
return apply_scalar_unary<inv_logit_fun, Container>::apply(x);
}
template <typename Container,
require_container_bt<std::is_arithmetic, Container>* = nullptr>
inline auto inv_logit(const Container& x) {
return apply_vector_unary<Container>::apply(
x, [](const auto& v) { return v.array().logistic(); });
}This way the Eigen logistic function will be used for all arithmetic containers (even arbitrarily nested ones) |
Sorry, something went wrong.
|
apply_vector_unary also handles returning an Eigen::Matrix vs Eigen::Array to match the input type, which would resolve the current unit test failures |
Sorry, something went wrong.
Jenkins Console Log Machine information No LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focal CPU: G++: Clang: |
Sorry, something went wrong.
There was a problem hiding this comment.
Just a couple of doc changes, but otherwise LGTM!
Sorry, something went wrong.
| * Vectorized version of inv_logit() for std::vector's containing ad types. | ||
| * | ||
| * @tparam T type of container | ||
| * @param x container | ||
| * @tparam T type of std::vector | ||
| * @param x std::vector |
There was a problem hiding this comment.
This overload would also be used for Eigen types with fvars I think
Sorry, something went wrong.
| * @tparam T A type of either `std::vector` whose inner type inherits from | ||
| * `Eigen::DenseBase` or a type that directly inherits from `Eigen::DenseBase`. | ||
| * The inner scalar type must not have a `var` scalar type. | ||
| * @param x Eigen expression |
There was a problem hiding this comment.
This overload would also be for std::vector with inner types of double or std::vector
Sorry, something went wrong.
|
Note there is also the Eigen::internal::scalar_logistic_op which can be used for the scalar (double) implementation instead of the custom implementation using exp calls. Not sure if there is any noticeable advantage besides shortening the code by a few lines. |
Sorry, something went wrong.
|
@andrjohns ty those docs should be cleaned up now! @jachymb personally I don't know what the benefit of using Eigen's logistic function for scalars would be. For the vectorized version of the code, using the Eigen logistic function makes sense so we can use their SIMD backend. But for scalars we would want to check the performance and precision before using it. I'm fine with hand waving the vectorized check because using an Eigen internal function is almost always faster for vectors. |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM!
Sorry, something went wrong.
Jenkins Console Log Machine information No LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focal CPU: G++: Clang: |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
Modifies inv_logit to use the logistic function from Eigen. Also makes a new reverse mode specialization of inv_logit for Eigen matrices.
Tests
No new tests.
Checklist
Copyright holder: Jáchym Barvínek and 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
the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested