New v2.17.1 release for Math and Stan?

Developers
1

Given the patch @seantalts made to the Math library (thanks!), I’d like us to consider releasing a new version of the library soon. If users are seeing runtimes of 1/4 relative to v2.17.0, then I think it’s probably well worth the effort. We don’t really need to patch anything else; there’s also no backwards incompatibility.

Thoughts?

3 Likes
2

+1

Easy to say for me given that work is on others… however, it sounds like a very good idea to me.

3

I’d say it’s probably a good idea. There is one bug that I found in the development version of CmdStan that wasn’t in the 2.17.0 release version, but that’s a somewhat separate issue from the Stan library itself.

4

If the 2.17.1 release = master + 1 commit, I think it is fine. Some other stuff has been merged into develop that requires C++11, so that should wait for 2.18.x.

5

Good point.

Release v2.18.0?

6

I think I would favor master + 1 commit. I don’t think we want the last Stan that is compatible with a non-C++11 compiler to be slow.

7

I like your point, @bgoodri… but I can’t compile RStan 2.16 without a C++11 capable compiler. This may not be true for cmdstan, but in practice the 2.16 RStan release is already a C++11 beast. I have g++ 4.6 and Intel 2015 compilers; none of them worked. Only a g++ 6.3 got me RStan 2.16 building.

1 Like
8

That’s OK for doing once, but I don’t want to maintain C++03 and C++11 versions of Stan on an ongoing basis.

9

There’s actually a bit of effort needed to merge that one PR back into master. My vote is to just move forward given what @wds15 reported back.

10

Can’t we just revert 1 commit from master? I think @wds15 is experiencing a problem where I accidentally made StanHeaders 2.16 use a C++11 construct that isn’t present in StanHeaders > 2.16.

11

Mind trying? I tried, but I’m at the limit of my git fu.

12

I believe it is just

git checkout master
git checkout -b bugfix/revert_string
git revert -m 1 -n 6fc8e2a598ec1839f82deb0ba81853d0faad5df5 # has conflicts
# fix formatting of stan/math/prim/scal/err/check_2F1_converges.hpp in editor
# fix formatting of stan/math/prim/scal/err/check_3F2_converges.hpp in editor
git add stan/math/prim/scal/err/check_2F1_converges.hpp
git add stan/math/prim/scal/err/check_3F2_converges.hpp
git revert --continue

Running the unit tests now to make sure it is okay.

1 Like
13

Thanks! @seantalts, if @bgoodri has it all ready to go, is that something that’s easy to release?

14

Releases are easier but still not that easy.

1 Like
15

Encountered one test failure with clang-4.0 so far. Not sure if it was always that way.

16

Does anyone remember fixing an error like this when the checks use char*?

make -j1 test/unit/math/prim/scal/err/check_positive_test
clang++-4.0 -Wall -I . -isystem lib/eigen_3.3.3 -isystem lib/boost_1.64.0 -isystem lib/cvodes_2.9.0/include -std=c++1y -DBOOST_RESULT_OF_USE_TR1 -DBOOST_NO_DECLTYPE -DBOOST_DISABLE_ASSERTS -DBOOST_PHOENIX_NO_VARIADIC_EXPRESSION -stdlib=libc++ -Wno-unused-function -Wno-uninitialized -DGTEST_USE_OWN_TR1_TUPLE -DGTEST_HAS_PTHREAD=0 -isystem lib/gtest_1.7.0/include -isystem lib/gtest_1.7.0 -O3 -DGTEST_USE_OWN_TR1_TUPLE -DGTEST_HAS_PTHREAD=0 -isystem lib/gtest_1.7.0/include -isystem lib/gtest_1.7.0 -O3 -DNO_FPRINTF_OUTPUT -pipe  -c -o test/unit/math/prim/scal/err/check_positive_test.o test/unit/math/prim/scal/err/check_positive_test.cpp
In file included from test/unit/math/prim/scal/err/check_positive_test.cpp:1:
In file included from ./stan/math/prim/scal.hpp:50:
./stan/math/prim/scal/err/check_positive.hpp:24:54: error: ordered comparison between pointer and zero ('double (*)(const char *)' and 'int')
          if (!boost::is_unsigned<T_y>::value && !(y > 0))
                                                   ~ ^ ~
./stan/math/prim/scal/err/check_positive.hpp:66:50: note: in instantiation of member function 'stan::math::(anonymous namespace)::positive<double (const char *), false>::check' requested here
      positive<T_y, is_vector_like<T_y>::value>::check(function, name, y);
                                                 ^
test/unit/math/prim/scal/err/check_positive_test.cpp:8:19: note: in instantiation of function template specialization 'stan::math::check_positive<double (const char *)>' requested here
  EXPECT_NO_THROW(check_positive(function, "x", nan));
                  ^
1 error generated.
17

Yes! is_nan is not properly included somewhere so it finds an unrelated function with that name instead.

2 Likes
18

Commit SHA?

19

No idea. Can’t check right now.

20

It is the test file mentioned in the error