Broken header tests in stan-dev/math stan/math/rev/fun/ordered_constrain.hpp

Developers
1

rev/fun/orderec_constrain.hpp is failing header tests on my machine. I can’t tell why because everyting involved in the function is a double value where the test is failing with this error:

clang++ -std=c++1y -Wno-unknown-warning-option -Wno-tautological-compare -Wno-sign-compare -D_REENTRANT       -I lib/tbb_2019_U8/include -O0  -I . -I lib/eigen_3.3.3 -I lib/boost_1.69.0 -I lib/sundials_4.1.0/include    -DBOOST_DISABLE_ASSERTS      -c -std=c++1y -Wno-unknown-warning-option -Wno-tautological-compare -Wno-sign-compare -D_REENTRANT       -I lib/tbb_2019_U8/include -O0  -I . -I lib/eigen_3.3.3 -I lib/boost_1.69.0 -I lib/sundials_4.1.0/include -O0 -include stan/math/rev/mat/fun/cholesky_decompose.hpp test/dummy.cpp -o /dev/null
In file included from <built-in>:1:
./stan/math/rev/mat/fun/ordered_constrain.hpp:44:23: error: assigning to 'double' from incompatible type
      'stan::math::var'
      exp_x_[n - 1] = exp(x[n]);
                      ^~~~~~~~~
1 error generated.

The function hwere it’ breaking is here, with the relevant line highlighted:

  template <std::size_t size>
  Eigen::VectorXd operator()(const std::array<bool, size>& needs_adj,
                             const Eigen::VectorXd& x) {
    N_ = x.size();

    Eigen::Matrix<double, Eigen::Dynamic, 1> y(N_);
    if (N_ == 0) {
      return y;
    }

    exp_x_ = ChainableStack::instance_->memalloc_.alloc_array<double>(N_ - 1);

    y[0] = x[0];
    for (int n = 1; n < N_; ++n) {
      exp_x_[n - 1] = exp(x[n]);  // ***** error is here on line 44 *****
      y[n] = y[n - 1] + exp_x_[n - 1];
    }
    return y;
  }

Given everythnig here is double, where is the error coming from?

I’m pinging the devs who’ve touched this recently—it’s compltely different than when I originally wrote it: @bbbales2, @seantalts, @syclik, and @andrjohns and @stevebronder . It now involves a class and some complicated interaction with template parameters.

1 Like
2

Is this develop or is it a certain branch?

I was the one that wrote this code.

Yes this is a good mystery.

3

Oh maybe exp here is promoting to var?

4

Does it work if you include the cmath header or using std::exp? Because it looks like the only exp definition that it’s able to find is the overload for var

5

I switched to develop and they pass. So this is some issue introduced by the complex branch. Sorry to bother everyone—this one’s on me to sort out.

4 Likes
6

But what a fine ordered constrain implementation! With all sorts of fun template parameters! Too bad you don’t get to hang out and look at it more :P

1 Like
7

Yes! The problem is an include-what-you-use bug in ordered_constrain.hpp, which uses exp without ever including it. I think what happens is that the C99 compatibility function ::exp gets used—we used to have huge troubles with ambiguity until we defined versions in our own namespaces.

The fix is exactly as @andrjohns suggestions—include <cmath> and use std::exp. This also shows up in positive_ordered.

It was only tickled when I added a definition for exp(const complex<T>&). There are other problems caused by my branch and vectorization which I’ll have to deal with, but I won’t bore everyone with the details. This overloading business is sooo complicated in C++ due to its non-locality.

8

Bob I’m goofing around with complex and saw this issue. I think it’s something about calling math inside std and vice versa. I have a branch that I’m working on some stuff related to this