Feature/2729 vectorize abs

[bob-carpenter]

Summary

Adds vectorized signatures for abs function for integer, real, and complex-valued functions.

Comments

1. After this is merged, we should be able to deprecate fabs.
2. @SteveBronder might want to suggest more general signatures to take expression templates without copying, but the ones I have in place are functional.

Tests

Full testing of primitives and types in test/unit/math/prim/fun/abs_test.cpp and of autodiff using the test framework in test/unit/math/mix/fun/abs_test.cpp.

Side Effects

No.

Release notes

• added vectorized abs() for all argument types

Checklist

• Math issue fully vectorize abs for int, real, complex #2729

• Copyright holder: Simons Foundation

  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

  • 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)

• the code is written in idiomatic C++ and changes are documented in the doxygen

• the new changes are tested

[WardBrian]

Using this branch with stan-dev/stanc3#1195 (which exposes the signatures to the compiler)
causes expression tests to fail to compile: https://jenkins.flatironinstitute.org/blue/organizations/jenkins/Stan%2FStanc3/detail/PR-1195/2/pipeline/304/

It looks like the issue is when the input is an eigen expression of a complex matrix

[bob-carpenter]

Thanks, @WardBrian. I need help with the following to fix.

@SteveBronder and @rok-cesnovar: Is there doc for the template traits somewhere that'll let me figure out how to carve out the right argument constraints? My problem isn't with C++, it's with our template traits lib and what it does and what its naming conventions are.

@SteveBronder and @rok-cesnovar: Is there doc for testing requirements for the math lib so we don't run into downstream problems in integration like @WardBrian is reporting? We don't want to have to wait until integration with stanc3 to find bugs in the math lib. In particular, how can I test a math function does the right thing with generic matrix-like args? Does it have to work for expression templates? For blocking?

@SteveBronder and @rok-cesnovar: Where is the doc for all of the varmat stuff? There's a cryptic definition for abs in the rev/fun directory, but I don't understand it. I also don't understand how this is being tested. Is it now a requirement that every vectorized function works for varmat? Also, how do I make sure that arrays of varmat work?

[WardBrian]

We don't want to have to wait until integration with stanc3 to find bugs in the math lib. In particular, how can I test a math function does the right thing with generic matrix-like args? Does it have to work for expression templates? For blocking?

I would like to echo this is important and currently kind of a chicken-egg situation. The math CI does run the same expression tests as the compiler, but the expression tests require as input a list of signatures to test, and that list is generated by the compiler. This means the math library cannot test until the compiler is updated, so the best we can do at the moment is run experimental compiler branches against the experimental math branches, which is far from ideal

[rok-cesnovar]

Is there doc for the template traits somewhere that'll let me figure out how to carve out the right argument constraints? My problem isn't with C++, it's with our template traits lib and what it does and what its naming conventions are.

I think https://mc-stan.org/math/d8/de1/group__type__trait.html and https://mc-stan.org/math/d1/db9/group__require__meta.html
are all the general docs we have on that topic, but Steve would know more here. Is this what you are looking for or should we expand this more?

We don't want to have to wait until integration with stanc3 to find bugs in the math lib. In particular, how can I test a math function does the right thing with generic matrix-like args? Does it have to work for expression templates? For blocking?

There are expression requirements for functions with arguments that are Eigen types. They have to accept expressions and they have to not evaluate expressions multiple times. I think this is the only requirement that was added in the last versions or last few years.

But this is only a requirement if a function is exposed in the Stan language. If it is exposed, it MUST be able to accept expressions, otherwise Stan users would see real long and bad C++ errors. This is what happened two years ago when we enabled functions returning expressions: #1643 This is why we added this huge chunk of CI testing that is testing expressions.

We could, in theory, have Stan Math C++ functions that are not exposed in the Stan language and don't accept expressions. This is how Eigen works sometimes, where some functions don't accept expressions and you just need to .eval() them in that case. We probably don't want to have those, but I don't see a way of enforcing this policy for functions that are not exposed in the language. It would require either the developer having to write tests for expressions and put the burden of checking that on the reviewer.

The things that are probably not documented well enough are:

• the fact that we are enforcing this policy wrt expressions
• that fact that you can test a currently non-existent function if it works properly with expressions even if you don't have a modified stanc3:

python3 runTests.py test/expressions/ --only-function "foo(vector) => vector"

Running this will do all the expression tests for this signature of a function that is not currently in Stan without having to touch stanc3 at all.

I don't think there is a way of testing this in CI though before we try to expose it - I am definitely open to ideas we could try.

Is it now a requirement that every vectorized function works for varmat?

No. We turn "varmat" on in stanc3 manually for selected function. It is better if there are more of them, but it is not a requirement at all.

There's a cryptic definition for abs in the rev/fun directory, but I don't understand it.

The abs() implementation just uses fabs

math/stan/math/rev/fun/fabs.hpp

Line 70 in 43ec11b

template <typename VarMat, require_var_matrix_t<VarMat>* = nullptr>

which looking at it quickly support varmat.

I also don't understand how this is being tested.

I believe this line tests varmat stuff for abs(): https://github.com/stan-dev/math/blob/develop/test/unit/math/mix/fun/abs_test.cpp#L72

Also see https://mc-stan.org/math/df/d82/reverse_mode_types.html

[bob-carpenter]

Thanks, @rok-cesnovar. I'm afraid I need something more example-oriented than the API doc, because I don't understand the implications of the API doc (like what extending MatrixBase does or what var_value does). Then my problems are compounded by traits for scalar having non-standard meaning (complex numbers are scalars in the mathematical sense, but not in the Stan traits sense).

So what we need is something to help someone who can do basic C++ bridge the gap from the matrix calculus to a multivariate function implementation.

[rok-cesnovar]

@bob-carpenter so something like the following guide for distributions https://mc-stan.org/math/df/d67/new_distribution.html, but for regular math functions?

I think that is a good idea. We also have a ton of great resources in a bunch of places, like https://mc-stan.org/math/d1/d66/getting_started.html we just need to give it a bit more structure.

[bob-carpenter]

. something like the following guide for distributions

Yes, @rok-cesnovar. But after spending an hour or two talking to @SteveBronder about this yesterday, I don't think I'm going to be able to help with the doc or with future C++ programming--the gap between the current doc and my understanding of C++11 and the current Stan math conventions is just too great.

I sent Steve feedback on the first page, but it was pretty much one question per line and it just gets worse in the actual doxygen API doc. I also have no idea how to figure out analytic derivatives for what you are calling "var mat". So I don't know if even the request I'm making is reasonable. I'm happy to keep providing feedback on doc iterations until the doc approaches something I can understand or you get tired dumbing things down and fleshing things out to make me happy. Until then, I'll leave the C++ coding to people like you who understand C++11 and Stan math better than me.

[rok-cesnovar]

Given that #2737 was merged, I am going to close this one.