-
-
Notifications
You must be signed in to change notification settings - Fork 190
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
Use logistic function from eigen (based on jachymb's PR) #3160
Conversation
Can this use |
@andrjohns do you mean something besides the definition in prim that uses 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));
} |
Yeah the issue with the current definition is that it won't use the vectorised Eigen implementation for 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) |
|
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: |
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.
Just a couple of doc changes, but otherwise LGTM!
stan/math/prim/fun/inv_logit.hpp
Outdated
* 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This overload would also be used for Eigen types with fvars
I think
stan/math/prim/fun/inv_logit.hpp
Outdated
* @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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This overload would also be for std::vector
with inner types of double
or std::vector
Note there is also the |
@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 |
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.
LGTM!
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: |
Summary
Modifies
inv_logit
to use thelogistic
function from Eigen. Also makes a new reverse mode specialization ofinv_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
./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