New v2.17.1 release for Math and Stan?

maintenance

#21

It passes by accident with other compilers


#22

Ok, I just got home, this file is missing the stan is_nan include.


#23

Whoa. Thanks, @sakrejda. I don’t remember that error.


#24

The unit tests pass with.

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
git cherry-pick 35f6f86a4a

I didn’t run the distribution tests.


#25

I think I had to fix those tests when I did the hypergeometric function stuff b/c I had the new compiler at the time so this makes sense.


#26

Are we sure 2.17 doesn’t have any C++11 requirements? I think it was already supported and required by the makefiles, right?

Asking only because it would be slightly nicer to do less releases if that one is already tainted by C++11 in an important way (for whoever needs a release without C++11).


#27

I am pretty sure 2.17 just has -std=c++1y in the Makefiles but there wasn’t any code that doesn’t compile with the earlier standard.


#28

So is the point that RStan and PyStan would be able to support older compilers but not CmdStan? That seems fair, though not addressing @wds15 situation.

@jjramsey that bug was me, I’ll fix it.


#29

Even CmdStan 2.17.x will be fine if the user either edits the Makefile locally or supplies -std=c++99 in make/local.


#30
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
git cherry-pick 35f6f86a4a

@seantalts Can you run all the tests after doing this and if everything passes, redo it on the master branch and release 2.17.1?


#31

I don’t think we can do that on master - we already merged a different revert commit into develop, which would cause tons of problems next time we merged develop into master. I think the only way to do a 2.17.1 bug fix for the char* thing is to do a one-off branch, off master, that we do the above on and then tag as 2.17.1. That history will not be a part of master or develop's timeline.

Is that okay with everyone?


#32

Why can’t we do that on master? Or merge bugfix/revert_string into master?


#33

Then master and develop will both have a version of the commit that has the effect of reverting PR 584. Git will want to apply each of them when those two branches are merged.

We might be able to do this. That commit (and branch) had fixes to files that don’t exist in master, though, so I’m not sure how that will go. I can test it out.


#34
git checkout bugfix/revert_string
git merge -s recursive -Xtheirs --no-commit develop

yields

Automatic merge went well; stopped before committing as requested

#35

So that branch has another commit that reverts PR 584 on it (different from the two commits that revert it on develop), which is the first scenario above. I think the issue with this would be merging bugfix/revert_string into master (not develop as in your code above) and then merging develop into master.

Maybe we can cherry pick one or both of the commits that revert PR 584 on develop (576151e9d7324f40c9fa9b2a937dc8b416a2bd5a and cd18ad32a61201b05639a40407880f9117004fc0) into master and then test if develop can be merged into master successfully.


#36

I don’t see why merging develop into master after merging bugfix/revert_string into master is fundamentally different from git’s perspective then merging develop into bugfix/revert_string since bugfix/revert_string is branched from master.


#37

Woops, you’re right - forgot it was branched from master.

Are you sure the outcome is correct? I haven’t used those command-line options to automatically handle conflicts.


#38

Also I think we don’t want to cherry-pick, so never mind about that option: http://www.draconianoverlord.com/2013/09/07/no-cherry-picking.html . The link also explains how to use release branches. I guess ideally we would have known in advance that we wanted to fix 2.17.0 and put the char * hotfix there.


#39

Without -s recursive -Xtheirs there are many merge conflicts (but they mostly look like whitespace things rather than char* vs string things. With that, there are no merge conflicts.


#40

Yeah, I don’t trust that at all… The result there is that there are two commits that purport to revert 584 on master after that (and only one on develop).