vectorize atan2
Summary
Vectorized atan2 and added a unit test.
Tests
There was no unit test for atan2 so I added for the scalar and vectorized cases.
Release notes
The function atan2 is vectorized on both the inputs.
Checklist
-
[x] Math issue #2809
-
[x] Copyright holder: Sean Pinkney
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/)
-
[x] the basic tests are passing
- unit tests pass (to run, use:
./runTests.py test/unit) - header checks pass, (
make test-headers) - dependencies checks pass, (
make test-math-dependencies) - docs build, (
make doxygen) - code passes the built in C++ standards checks (
make cpplint)
- unit tests pass (to run, use:
-
[x] the code is written in idiomatic C++ and changes are documented in the doxygen
-
[x] the new changes are tested
@andrjohns 2 questions
stan::test::expect_common_nonzero_binary(f);this errors out foratan2, do you know why?- On the 2nd test in that beta file if I do it on
atan2I'm getting this error (as well as a bunch after)test/unit/math/mix/fun/atan2_test.cpp:42:12: error: no matching function for call to 'atan2' return atan2(x1, x2);
So the 2nd error is caused by the expect_ad_matvar test, because the atan2 function doesn't have an overload for var_value<Eigen> types, like the beta function, so you can just remove those tests.
What was the expect_common_nonzero_binary error? I can't find it in the logs above sorry
Yes, I just added the overload :). I thought it would just work without it, lol. Once I push we'll be able to see the expect_common_nonzero_binary errors.
The first error that comes up is
./test/unit/math/expect_near_rel.hpp:77: Failure
Value of: stan::math::is_nan(x1) && stan::math::is_nan(x2)
Actual: false
Expected: true
expect_near_rel(0, nan)
expect_near_rel; require items x1(0) = x2(0): gradient() grad for finite diff vs auto diff
./test/unit/math/expect_near_rel.hpp:77: Failure
Value of: stan::math::is_nan(x1) && stan::math::is_nan(x2)
Actual: false
Expected: true
expect_near_rel(0, nan)
expect_near_rel; require items x1(0) = x2(0): gradient_fvar() grad
I checked the values being used by the expect_common_nonzero_binary function, and it looks like the error occurs when the first argument to atan2 is infinite. You can reproduce with:
stan::test::expect_ad(f, stan::math::INFTY, 1.0);
I would guess that there needs to be a check for finite values in the gradient calculations, otherwise one of the intermediate calculations results in a nan being returned
That makes sense but I don't know how to achieve this. I tried a bunch of combinations of
if (is_inf(a.val()) || is_nan(a.val()) || is_inf(b.val()) || is_nan(b.val())) {
return make_callback_var(
std::atan2(a.val(), b.val()), [a, b](const auto& vi) mutable {
a.adj() += 0;
b.adj() += 0;
});
}
which doesn't solve the issue. Plus, I'm not even sure that should work or is "idiomatic c++". I think I'll have to leave this unless you or someone else can fix the issue.
@andrjohns I think I'll comment out expect_common_nonzero_binary and just test the derivative manually. Talking to @rok-cesnovar it seems that finite differences is failing for infinite input (as you noted too). However, I can't figure out how to just test expect_equal to nan for a derivative.
@andrjohns I think I'll comment out
expect_common_nonzero_binaryand just test the derivative manually. Talking to @rok-cesnovar it seems that finite differences is failing for infinite input (as you noted too). However, I can't figure out how to just test expect_equal to nan for a derivative.
Sounds good to me! Ping me when this is ready for another look
I can't figure out how to just test expect_equal to nan for a derivative.
@andrjohns, would you be able to help with this?
This is good to go now, right?