math.h vs. cmath

[Bob Carpenter]

[moved back to stan-dev]

Are you importing <math.h> somewhere rather than <cmath>? The
former is the old C-compatiblity stuff that imports into the
top-levle namespace ::, whereas the latter puts things in std::
namespace.

Ack. Looks like we're importing <math.h> instead of <cmath>,
which has the problem of bringing things into the top-level ::
namespace rather than std::.

So I created a new issue:

https://github.com/stan-dev/math/issues/277

But this doesn't help you on this pull request. I don't know why
it's failing now unless you bring in math.h inadvertently yourself
somewhere in the <math.hpp> header or elsewhere.

I don't know why these are all math.h, because the functions
(or at least most of them) are in <cmath>.

~/cmdstan/stan/lib/stan_math((bbe2de0...))$ grep -R "math.h" stan
stan/math/fwd/scal/fun/acosh.hpp:#include <math.h>
stan/math/fwd/scal/fun/asinh.hpp:#include <math.h>
stan/math/fwd/scal/fun/atanh.hpp:#include <math.h>
stan/math/fwd/scal/fun/cbrt.hpp:#include <math.h>
stan/math/fwd/scal/fun/erf.hpp:#include <math.h>
stan/math/fwd/scal/fun/erfc.hpp:#include <math.h>
stan/math/fwd/scal/fun/expm1.hpp:#include <math.h>
stan/math/fwd/scal/fun/fma.hpp:#include <math.h>
stan/math/fwd/scal/fun/fmax.hpp:#include <math.h>
stan/math/fwd/scal/fun/fmin.hpp:#include <math.h>
stan/math/fwd/scal/fun/hypot.hpp:#include <math.h>
stan/math/fwd/scal/fun/round.hpp:#include <math.h>
stan/math/fwd/scal/fun/trunc.hpp:#include <math.h>
stan/math/prim/scal/fun/fdim.hpp:#include <math.h>
stan/math/rev/scal/fun/acosh.hpp:#include <math.h>
stan/math/rev/scal/fun/asinh.hpp:#include <math.h>
stan/math/rev/scal/fun/atanh.hpp:#include <math.h>
stan/math/rev/scal/fun/cbrt.hpp:#include <math.h>
stan/math/rev/scal/fun/erf.hpp:#include <math.h>
stan/math/rev/scal/fun/erfc.hpp:#include <math.h>
stan/math/rev/scal/fun/expm1.hpp:#include <math.h>
stan/math/rev/scal/fun/fma.hpp:#include <math.h>
stan/math/rev/scal/fun/hypot.hpp:#include <math.h>
stan/math/rev/scal/fun/owens_t.hpp:#include <math.h>
stan/math/rev/scal/fun/round.hpp:#include <math.h>
stan/math/rev/scal/fun/trunc.hpp:#include <math.h>.

- Bob


> On Apr 5, 2016, at 6:47 PM, Sebastian Weber <sdw....@gmail.com> wrote:
>
> Hi Daniel!
>
> I just checked and Jenkins barfs with this:
>
> Wall -DBOOST_RESULT_OF_USE_TR1 -DBOOST_NO_DECLTYPE -DBOOST_DISABLE_ASSERTS -pipe -Ilib/stan_math/lib/cvode_2.8.2/include -Wno-unknown-warning-option -Wno-unused-function -Wno-tautological-compare -Wno-c++11-long-long -Wsign-compare -Wno-unused-local-typedef -ftemplate-depth=256 -c -fsyntax-only -O0 src/test/test-models/good/function-signatures/math/functions/exp2.hpp
> src/test/test-models/good/function-signatures/math/functions/exp2.hpp:65:55: error: call to 'exp2' is ambiguous
> stan::math::assign(transformed_data_real, exp2(d_int));
> ^~~~
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/usr/include/math.h:379:15: note: candidate function
> extern double exp2(double);
> ^
> lib/stan_math/stan/math/prim/scal/fun/exp2.hpp:19:19: note: candidate function
> inline double exp2(const double y) {
> ^
>
>
> However, this has nothing to do with the changes I made??? Is this familiar to you or where should I start to dig?
>
> Sebastian
>
>
>> Am 05.04.2016 um 21:40 schrieb Daniel Lee <bea...@alum.mit.edu>:
>>
>> It should be fine... it failed due to me trying to get GitHub to spit out the correct message. So it was something I changed.
>>
>> On Tue, Apr 5, 2016 at 3:36 PM, Sebastian Weber <sdw....@gmail.com> wrote:
>> Hi Daniel!
>>
>> Thanks for checking and sorry for the noise. If something goes wrong, just let me know; hopefully quick to fix.
>>
>> Sebastian
>>
>> On Tue, Apr 5, 2016 at 9:34 PM, Daniel Lee <bea...@alum.mit.edu> wrote:
>> Hi Sebastian,
>>
>> The tests are correct. You can see the revision is correct: 38cfe98
>>
>> I'm not 100% certain how the changes are populated, but I believe those are changes with respect to your branch. Those all have to be merged into your branch for it to be caught up to develop.
>>
>>
>>
>> Daniel
>>
>>
>>
>>
>> On Tue, Apr 5, 2016 at 3:28 PM, Sebastian Weber <sdw....@gmail.com> wrote:
>> Hi Daniel!
>>
>> I am concerned that Jenkins does test the wrong branch for the robust integrator pull. That is, if I click on "Details" on the currently running Jenkins initiated test which is listed in the pull request for the robust integrator. Then I end up here:
>>
>> http://d1m1s1b1.stat.columbia.edu:8080/job/Math%20Pull%20Request/287/
>>
>> The changes which have happened according to that Jenkins page are:
>>
>> Changes
>> • ensure that usual Jacobian (and not a transposed one) is returned (commit: 4a4ef64febe8857d7cd0a0b9eb701103297d132a) (detail / githubweb)
>> • Removing compiler warning (commit: 3d18aa9293e28c7c082b65261138acd490e797a1) (detail / githubweb)
>> • corrected autodiff_test to test for correctly returned output from (commit: d1dfae12e3622c7f7a0d9ee1e61483aab4c088de) (detail /githubweb)
>> • fixed fwd version of jacobian and corrected test (commit: 844373c89d7d2d8c17dcbc8d434e3679858f938c) (detail / githubweb)
>> Which is the old, already merged Jacobian transposed story. I am confused!
>>
>> If this is correct behavior, then please ignore this mail.
>>
>> Best,
>> Sebastian
>>
>>
>>
>
>

[Rayleigh L]

Hi,

That error message looks like the Jenkins error message that I saw yesterday for my pull request to vectorize unary functions. However, my changes are on my branch and not on develop.

Best,

Rayleigh

--
You received this message because you are subscribed to the Google Groups "stan development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Daniel Lee]

Yes. I don't know how that error ended up on a different pull request. I need to find out what's going on with Jenkins. 

Daniel

[Bob Carpenter]

What you need to do for your pull request is figure
out where the double import is coming from. Which file
did you add that causes the problem?

- Bob

> On Apr 5, 2016, at 7:56 PM, Rayleigh L <rayleig...@gmail.com> wrote:
>

[Ben Goodrich]

iwyu may figure it out for you

> To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+unsubscribe@googlegroups.com.

> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> You received this message because you are subscribed to the Google Groups "stan development mailing list" group.

> To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+unsubscribe@googlegroups.com.

[Rayleigh L]

Which pull request are we talking about? If we're talking about my pull request to add vectorizing unary functions, I noticed that the functions causing problems are those that:

1) Are untemplated in stan/math/prim/scal/fun and

2) Are externed in math.h (e.g. extern double exp2(double) in math.h)

I tried removing the prim version of exp2 today, but it looks like exp2 becomes part of cmath only for C++11. Also, I think the only other functions with this issue are log2 and log1p, but I can't verify because of a segfault error when testing stan-headers for functions. Those two functions also become part of cmath for C++11.

> To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+u...@googlegroups.com.

> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> You received this message because you are subscribed to the Google Groups "stan development mailing list" group.

> To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+u...@googlegroups.com.

> For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "stan development mailing list" group.

To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+u...@googlegroups.com.

[Daniel Lee]

I believe it's that pull request that's adding failures. Unless...

Ben, did you change the compiler on that Windows machine?

[Daniel Lee]

wait... it's not even on the Windows machine. Ignore my last bit of my email.

[Daniel Lee]

Rayleigh, here's how to recreate what's going on Jenkins. I was able to recreate it locally.

Hopefully you can follow these steps. I'm going to have you clone two instances of stan so you can keep one on develop.

1. Clone the first instance of Stan:

    git clone https://github.com/stan-dev/stan stan-develop

2. Update the submodules

    cd stan-develop

    git submodule update --recursive --init

    (you might not need recursive since math doesn't have submodules)

3. Run the test that passes on develop:
    make src/test/test-models/good/function-signatures/math/functions/exp2.hpp-test

4. Clone the second instance of Stan:
    git clone https://github.com/stan-dev/stan stan-202

5. Update the submodule

    cd stan-202

    git submodule update --recursive --init

    pushd lib/stan_math

    git checkout feature/issue-202-vectorize-all

    popd

6. Run the test that fails with your branch:
    make src/test/test-models/good/function-signatures/math/functions/exp2.hpp-test

So, you'll notice steps 3 and 6 show the problem.

One last hint to help you find out what's causing the problem: use the -M option on the compiler. If you see the clang++ call for
    make src/test/test-models/good/function-signatures/math/functions/exp2.hpp-test

it's
    clang++ -I src -isystem lib/stan_math/lib/eigen_3.2.8 -isystem lib/stan_math/lib/boost_1.60.0 -isystem lib/stan_math/ -Wall -DBOOST_RESULT_OF_USE_TR1 -DBOOST_NO_DECLTYPE -DBOOST_DISABLE_ASSERTS -pipe -Ilib/stan_math/lib/cvode_2.8.2/include -Wno-unknown-warning-option -Wno-unused-function -Wno-tautological-compare -Wno-c++11-long-long -Wsign-compare -Wno-unused-local-typedef -ftemplate-depth=256   -c -fsyntax-only -O0 src/test/test-models/good/function-signatures/math/functions/exp2.hpp

Add a -M option to that (at the end, after exp2.hpp, is fine). You should see something like:

clang: warning: argument unused during compilation: '-fsyntax-only'
exp2.o: \
  src/test/test-models/good/function-signatures/math/functions/exp2.hpp \
  src/stan/model/model_header.hpp lib/stan_math/stan/math.hpp \
  lib/stan_math/stan/math/rev/mat.hpp \
  lib/stan_math/stan/math/rev/core.hpp \

It's going to be tricky -- I just looked and it's not obvious what's different. You might also want to look at the generated exp2.hpp file. Perhaps we're not generating the C++ in the right way?

Daniel

[Bob Carpenter]

We can't use C++11 yet. I thought there were a couple of holdouts.

(1) and (2) are clues enough this should be possible to track down.

> On Apr 5, 2016, at 11:05 PM, Rayleigh L <rayleig...@gmail.com> wrote:
>
> Which pull request are we talking about? If we're talking about my pull request to add vectorizing unary functions, I noticed that the functions causing problems are those that:
>
> 1) Are untemplated in stan/math/prim/scal/fun and

Did you mean "or" here? exp2 is templated in prim/scal/fun/exp2.hpp:

template <typename T>
inline typename boost::math::tools::promote_args<T>::type
exp2(const T y) {
using std::pow;
return pow(2.0, y);
}

I also think these should be const T& not just const T if
they are going to accept var and fvar inputs.

Another hint is that the lib only defines a double version, not
an int version.

C++11 then overloads for integer types with T. Are you compiling
with C++11 flags on by any chance?

See: http://www.cplusplus.com/reference/cmath/exp2/

> 2) Are externed in math.h (e.g. extern double exp2(double) in math.h)
>
> I tried removing the prim version of exp2 today,

You can also try defining it for double values instead of having
it be templated.

> but it looks like exp2 becomes part of cmath only for C++11. Also, I think the only other functions with this issue are log2 and log1p, but I can't verify because of a segfault error when testing stan-headers for functions.

What do you mean by stan-headers here? The R package?

> Those two functions also become part of cmath for C++11.

We can't use C++11 yet.

- Bob

[Rayleigh L]

Hi,

I wasn't clear in my last email. I have been able to recreate the issue locally using what you pointed out, Daniel. However, when I try running make src/test/test-models/good/function-signatures/math/functions/exp2.hpp-test after pulling the latest develop version of Stan, I get a segmentation fault error.

As for your question, Bob, I meant and. On the develop branch, exp2 is templated, but on my branch/pull request (issue-202/vectorize-all), it's not in order to vectorize it. From what I remember about the error message, it looked to be failing because extern double exp2(double) in math.h is ambiguous. That call finds two candidate functions: the rev version of exp2 and the untemplated prim version of exp2. When I re-templated exp2 and ran make src/test/test-models/good/function-signatures/math/functions/exp2.hpp-test on Monday, I didn't get an error message. I tried removing the prim version of exp2 as another way to resolve the ambiguity, but as pointed above, I ran into the issue that exp2 is only part of C++11's cmath.

Best,

Rayleigh

[Bob Carpenter]

You need to work through this on first principles and
track down where the ambiguity is. We can help you do
it after the 2.10 release if you haven't figured it out
by then.

I didn't realize you untemplated it. exp2(double) in math.h isn't
ambiguous by itself. If it were, nobody would be able to use it!
So something else is being included that causes the ambiguity.
And my guess is that you have the definition in stan::math and
the definition in :: from math.h. You need to solve this by making
sure both are not brought in. You can do that in a couple of ways.
It's not critical that we define a primitive version in stan::math.
The one from the library can be included.

All this talk about C++11 is confusing me. We are NOT moving to
C++11 yet, so we CANNOT introduce dependencies on C++11. But we
might be testing for C++11 compatibility (I'm not sure any more --- it's
been on and off the table since the project started; once these
replies get this deep if you want answers, you'll need to dig yourself
or ask someone else --- nobody else is still reading). If we are
testing for C++11 compatibility, that produces further constraints on
what you can do.

But for now, we MUST bring in a different implementation other than
the C++11 definition, so the question is whether to bring one in
from Boost or by defining one ourselves. In the past, we've defined
exp2 ourselves. There's no reason that basic definition needs to
be templated. It does need to work for int and double.

So what you need to do is track down where the ambiguity is being
brought in. It's probably from too many using statements, etc.

If you're segfaulting in a test, figure out where by instrumenting
with print statements.

- Bob