Stan 2.6.2?

[Bob Carpenter]

Whatever happened to Stan 2.6.2? I kept trying to bring it
up in the meeting, but I couldn't get a response (it's tough
being on a slow remote connection).

Is there a set of pull requests that need to be finished?
Are we still waiting on that 128-bit double fix?

Am I supposed to be doing something that I haven't done yet?

I'm not in a huge rush, but I thought CRAN and RStudio and
what not were waiting on it. It also has some bug fixes we'd
like to roll out.

- Bob

[Daniel Lee]

On Thursday, March 12, 2015, Bob Carpenter <ca...@alias-i.com> wrote:

Whatever happened to Stan 2.6.2?  I kept trying to bring it
up in the meeting, but I couldn't get a response (it's tough
being on a slow remote connection).

Yes, we'll tag tomorrow. 

Is there a set of pull requests that need to be finished?
Are we still waiting on that 128-bit double fix?

Since. They are on the queue,  

1357 normal cdf log's derivatives

1173 Eigen 3.2.4

It'll be a day before its done (~20 hrs assuming no hiccups).

We can tag without those two.

The 128-bit double fix: Ben, can you check to see if models still compile under Linux using g++ 4.9? We should think about end-to-end tests across different platforms. Maybe we can set up Travis to do that for us.

Am I supposed to be doing something that I haven't done yet?

No. If you want to release based on what we have now, I'm comfortable with it. Ben and Jiqiang, if you need eigen to be updated for CRAN, we can wait for the update.

 

I'm not in a huge rush, but I thought CRAN and RStudio and
what not were waiting on it.  It also has some bug fixes we'd
like to roll out.

We won't sneak more into the release. The only 3 fixes that are on the table are:

1. Bug fix for derivative of normal cdf log and variants; already on testing queue

 And is being tested now.

2. Eigen upgrade to 3.2.4. This is also on the testing queue.

3. Fix for 128-bit double constructor which makes Linux g++ 4.9 fail, as far as I know. Waiting for confirmation and a fix.

 

- Bob




--
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.

[Bob Carpenter]

>
> We won't sneak more into the release. The only 3 fixes that are on the table are:
> 1. Bug fix for derivative of normal cdf log and variants; already on testing queue
> And is being tested now.
> 2. Eigen upgrade to 3.2.4. This is also on the testing queue.
> 3. Fix for 128-bit double constructor which makes Linux g++ 4.9 fail, as far as I know. Waiting for confirmation and a fix.
>

I'll put a patch in for (3), but someone will need to test
it on Linux as I don't have access to a linux machine that I know
how to run.

- Bob

[Ben Goodrich]

On Thursday, March 12, 2015 at 1:11:49 AM UTC-4, Daniel Lee wrote:

The 128-bit double fix: Ben, can you check to see if models still compile under Linux using g++ 4.9?

I am up to date with develop and ./runTests.py src/test/unit/math/mix/mat/prob/lkj_corr_test.cpp still fails with g++-4.9 only.
 

No. If you want to release based on what we have now, I'm comfortable with it. Ben and Jiqiang, if you need eigen to be updated for CRAN, we can wait for the update.

I think Stan should update to Eigen 3.2.4 before the release because that is what RcppEigen is on. But we'll have to use RcppEigen's version even if Stan doesn't upgrade now.

Ben

[Bob Carpenter]

I just put in the pull request for Eigen 3.2.4.

I'll get the pull request in for g++ 4.9 in a couple minutes; just
as soon as I can sort through the compiler flags.

- Bob

[Daniel Lee]

Thanks, Bob. 

Ben, that can't possibly be an include split problem, right? Just to double check, can you include <stan/model/model_header.hpp> at the top of the test? I'm expecting this to do nothing to fix the problem, but if it does, then it's a different fix than what Bob is envisioning.

On Thursday, March 12, 2015, Bob Carpenter <ca...@alias-i.com> wrote:

[Ben Goodrich]

On Thursday, March 12, 2015 at 1:45:11 AM UTC-4, Daniel Lee wrote:

Thanks, Bob. 

Ben, that can't possibly be an include split problem, right? Just to double check, can you include <stan/model/model_header.hpp> at the top of the test? I'm expecting this to do nothing to fix the problem, but if it does, then it's a different fix than what Bob is envisioning.

I think it is a var constructor thing. That results in a different error.

In file included from src/test/unit/math/mix/mat/prob/lkj_corr_test.cpp:9:0:
src/stan/math/fwd/mat/fun/Eigen_NumTraits.hpp: At global scope:
src/stan/math/fwd/mat/fun/Eigen_NumTraits.hpp:15:10: error: partial specialization of ‘Eigen::NumTraits<stan::agrad::fvar<T> >’ after instantiation of ‘Eigen::NumTraits<stan::agrad::fvar<stan::agrad::var> >’ [-fpermissive]
   struct NumTraits<stan::agrad::fvar<T> > {
          ^
src/stan/math/fwd/mat/fun/Eigen_NumTraits.hpp:15:10: error: partial specialization of ‘Eigen::NumTraits<stan::agrad::fvar<T> >’ after instantiation of ‘Eigen::NumTraits<stan::agrad::fvar<stan::agrad::fvar<stan::agrad::var> > >’ [-fpermissive]

Ben

[Daniel Lee]

I didn't look carefully enough. 

You're running the mix tests. We don't actually have any fvar and var code in our generated models. If there's a rev version of the test, can you try that? And can you also try the same test again, but put the include of model header right before the include to lkj_corr? 

In order for us to instantiate the meta programs in the right way, fvar needs to be brought in before var, I think.

[Bob Carpenter]

> On Mar 12, 2015, at 4:57 PM, Daniel Lee <bea...@alum.mit.edu> wrote:
>
> I didn't look carefully enough.
>
> You're running the mix tests. We don't actually have any fvar and var code in our generated models. If there's a rev version of the test, can you try that? And can you also try the same test again, but put the include of model header right before the include to lkj_corr?
>
> In order for us to instantiate the meta programs in the right way, fvar needs to be brought in before var, I think.

I would guess that's the other way around --- fvar<var> depends on var, but nothing
in var depends on fvar.

- Bob

[Bob Carpenter]

Ben --- can you validate that adding the following to

src/stan/math/rev/core/var.hpp

after the Windows ifdef will allow you to compile:

#if (__GNUC__ > 4) || ((__GNUC__ == 4) && (__GNUC__MINOR >= 3))
// in GNUC 4.6 and beyond, we need to allow __float128
// due to some combination of Eigen and Boost
var(__float128 x) : vi_(new vari(static_cast<double>(x))) { }
#endif

The other tack we could take would be fixing lkj_corr, which seems
to be the source of the issue.

It just gets ignored on my Mac so there's no point putting in a
pull request until we can make sure it works on the platforms its
intended to fix. It now seems we need to get linux into our auto-tested
stack, and as far as that goes, we really need to test multiple
versions of GCC. What a mess!

I'm a bit worried by

https://gcc.gnu.org/onlinedocs/gcc/Floating-Types.html

which states

Not all targets support additional floating-point types.
__float80 and __float128 types are supported on x86 and IA-64
targets. The __float128 type is supported on hppa HP-UX targets.

I have no idea if anyone (including CRAN) will be testing hardware
other than Intel.

- Bob

[Ben Goodrich]

There is only prim, mix, and fwd

goodrich@T540p:/opt/stan$ find src/ -type f -name "lkj_corr_test*"
src/test/unit/math/prim/mat/prob/lkj_corr_test.cpp
src/test/unit/math/fwd/mat/prob/lkj_corr_test.cpp
src/test/unit/math/mix/mat/prob/lkj_corr_test.cpp

but including model_header right before lkj_corr (line 37) makes it pass.

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

[Ben Goodrich]

On Thursday, March 12, 2015 at 2:18:17 AM UTC-4, Bob Carpenter wrote:

Ben --- can you validate that adding the following to

  src/stan/math/rev/core/var.hpp

after the Windows ifdef will allow you to compile:

#if (__GNUC__ > 4) || ((__GNUC__ == 4) && (__GNUC__MINOR >= 3))
      // in GNUC 4.6 and beyond, we need to allow __float128
      // due to some combination of Eigen and Boost
      var(__float128 x) : vi_(new vari(static_cast<double>(x))) { }
#endif

It doesn't complain. But it yields the same (or similar) compiler error (attached).
 

I'm a bit worried by

  https://gcc.gnu.org/onlinedocs/gcc/Floating-Types.html

which states

  Not all targets support additional floating-point types.  
  __float80 and __float128 types are supported on x86 and IA-64
  targets. The __float128 type is supported on hppa HP-UX targets.

I have no idea if anyone (including CRAN) will be testing hardware
other than Intel.

CRAN still tests on solaris and solaris-sparc but those usually behave like Linux.

Ben

[Bob Carpenter]

Could you try it without the #if and see if that works? Then we can
work backwards to figure out what the #if needs to be.

It's very hard for me to debug from here because I don't have any
problems compiling.

The alternative is to fix lkj_corr so it's not calling or including
whatever the offending function is.

- Bob

> --
> 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.

> <error.txt>

[Bob Carpenter]

The proximate cause here seems to be fvar's use of
boost::math::isnan() to set the tangent to NaN. But
it would probably creep in elsewhere, because I don't see
why that one's special. I was hoping we could just get rid
of the problem, which we may be able to do by not using
Boost's isnan anywhere. I'm getting lost in all the refactoring,
but my understanding was that we were going to get rid of
Boost's isnan() and use our own.

Ben, if including this in var.hpp with no #ifdef

var(__float128 x) : vi_(new vari(static_cast<double>(x))) { }

lets you compile, can you try this:

#ifdef BOOST_MATH_USE_FLOAT128

var(__float128 x) : vi_(new vari(static_cast<double>(x))) { }
#endif

That may introduce an order dependency, and if it doesn't work,
maybe a spell-corrected version of my first attempt would work (I forgot
the final "__" on "__GNUC_MINOR__"):

#if (__GNUC__ > 4) || ((__GNUC__ == 4) && (__GNUC_MINOR__ >= 3))

var(__float128 x) : vi_(new vari(static_cast<double>(x))) { }
#endif

That should not depend on Boost --- if we use the Boost property,
then we may have more ordering issues.


The offending bit of Boost code that enables the non-standard
type __float128 is here:

lib/boost_1.55.0//boost/math/tools/config.hpp

which has:

#if defined(_GLIBCXX_USE_FLOAT128) && defined(BOOST_GCC) && !defined(__STRICT_ANSI__)
//
// Only enable this when the compiler really is GCC as clang and probably
// intel too don't support __float128 yet :-(
//
# define BOOST_MATH_USE_FLOAT128
#endif


I'd go back to trying enable_if, but I couldn't get it to work on
Windows --- the tests all passed here and crashed and burned on Jenkins.

- Bob

[Daniel Lee]

Ouch.

Ok... that indicates something else. I'll need to get g++ 4.9 to hunt this one down.

Daniel

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

[Ben Goodrich]

On Thursday, March 12, 2015 at 4:49:18 AM UTC-4, Bob Carpenter wrote:

Ben, if including this in var.hpp with no #ifdef

      var(__float128 x) : vi_(new vari(static_cast<double>(x))) { }

lets you compile,

Yes, that works.
 

can you try this:

#ifdef BOOST_MATH_USE_FLOAT128
      var(__float128 x) : vi_(new vari(static_cast<double>(x))) { }
#endif

That also works.

That may introduce an order dependency, and if it doesn't work,
maybe a spell-corrected version of my first attempt would work (I forgot
the final "__" on "__GNUC_MINOR__"):

#if (__GNUC__ > 4) || ((__GNUC__ == 4) && (__GNUC_MINOR__ >= 3))
      var(__float128 x) : vi_(new vari(static_cast<double>(x))) { }
#endif

That didn't work.

Ben

[Bob Carpenter]

Thanks! I'll put in the patch and make sure to deal
with the order dependency properly.

- Bob

[Daniel Lee]

Bob, I was able to set up a virtual machine with g++ 4.9.

The weird thing is that the patch isn't needed if we #include <stan/model/model_header.hpp> right before including <stan/math/prim/mat/prob/lkj_corr.hpp>

[Bob Carpenter]

That's very weird. I'm wondering if there's something
odd going on where we would accidentally call the __float128
type?

I also wonder why it's just lkj_corr that's causing issues.

- Bob

[Daniel Lee]

Ok... I narrowed it down to need a header inside <stan/math/rev/scal.hpp>.

(I thought it'd be in mat, but what do I know? template metaprogramming is tricky. I'll let you know exactly which header we needed to include.)

[Daniel Lee]

Ok. Found it!

Ben, can you include this header:
#include <stan/math/rev/scal/fun/digamma.hpp>

right before the include to lkj_corr? It doesn't need to be right there, but that seems to work just fine. I also put it closer to the top and it works.

Bob, if this works, I think we should fix it by including this into the test rather than macros for the includes. This is really just a mix autodiff problem, forward and backwards interacting together.

[Bob Carpenter]

Well, feel free to do what you want with my messed up
git merge. Was that why you were asking the last question?

Why do you want to include digamma in lkj_corr? That seems
like the wrong fix unless it uses the function.

- Bob

[Daniel Lee]

I'll just set develop back to where it was.

You're absolutely right on the digamma comment. I'm now trying to hunt down which piece of that will make this function work.

[Daniel Lee]

It looks like it's necessary.

I'll try to find out what part of boost calls it. It's getting implicitly instantiated somehow, so we need this digamma function, I'm just not sure where.

[Bob Carpenter]

We definitely need the digamma for derivatives of
the gamma function. It's also a pain for higher-order
derivatives, because you then need trigramma, and then
I'm not sure what for third order derivatives.

My question is whether lkj_corr needed it. I guess
it does because it uses beta distributions and they
implicate gamma, which in turn implicates digamma.

- Bob

[Daniel Lee]

Thanks, Bob.

could source of the problem be stan/math/rev/scal/fun/lgamma.hpp? It calls boost directly.

[Bob Carpenter]

> On Mar 13, 2015, at 2:39 AM, Daniel Lee <bea...@alum.mit.edu> wrote:
>
> Thanks, Bob.
>
> could source of the problem be stan/math/rev/scal/fun/lgamma.hpp? It calls boost directly.

Yes, that's how we compute gamma and digamma.

The <valarray> and <...constants.hpp> includes seem unnecessary.

I'm not even sure what you mean by "source of the problem."
Can you compile and test lgamma? If so, then it's not by itself
the source of the problem.

I also thought it was the forward version acting up?

That itself also brings in way more than necessary with all of
fwd/core.hpp.

- Bob

[Daniel Lee]

So... Bob, I think you should add your patch.

But I also think that the test is missing includes for digamma, based on how it's being used. I'll go ahead and add some includes and submit a pull request.

Daniel