rok-cesnovar commented 4 years ago

Based on our schedule the next release is set to happen on the 19th of October, which means that the feature freeze will begin on the 12th of October (next Monday).

Release notes will be generated using the Github Actions job: https://github.com/stan-dev/math/actions?query=workflow%3A%22Release+scripts%22 We only need to do some copy edit afterwards (list all generalize changes together for example). If no one will volunteer for the copy-edit, I will do that on the 13th.

I would like to discuss how we handle the feature freeze. Would it be possible to do the feature freeze by merging develop at that point to master and tagging that as release candidate? We could then continue merging to develop as normally even during the feature freeze. If any issues come up with the RC during the freeze period we would make a bugfix PR to master and not develop. So master would then be RC+bugfixPRs

The reason I am asking this is that we do 4 releases a year and doing a full stop of operations on Math for a week per release means that we lose a month/year that way. And in periods of active development (like the last release for example), this can also cause huge backlogs of PRs that are waiting to be merged. And we then have PRs that are done to branches and that can get tricky really fast.

Dont think this is necessary for other repositories (Stan and Cmdstan certainly arent as active), but Math is currently in a huge transition and I think this would be worthwhile.

wds15 commented 4 years ago

How can then the bug fixes to the RC be back merged to develop in a clean way?

rok-cesnovar commented 4 years ago

Would a PR merging master to develop after the actual release be considered clean?

wds15 commented 4 years ago

Maybe. This is a question about keeping git history nice. I recall that we had long discussions over it...let's ping @seantalts for his wisdom here.

seantalts commented 4 years ago

doing a full stop of operations on Math for a week per release

We shouldn’t need a full stop, just to hold off on merging things to develop. Development can continue but we lose synchronization for a week each time.

Math is currently in a huge transition

Fair enough, synchronization is important in those cases.

The value of the feature freeze is that people are using and developing off the frozen branch for a while. If we change it to the proposed master merge, we lose the value of the freeze. Unless you all are aware of some people using the master branch for their work before it is released?

So perhaps it is not worth doing a feature freeze during this period of math activity? Though it seems like periods of heavy development are those that most need some additional testing. Maybe better to skip this release?

Would a PR merging master to develop after the actual release be considered clean?

It wouldn’t automatically merge cleanly, but you could resolve any conflicts by hand. Depends on what’s happened in develop since master was pulled off.

SteveBronder commented 4 years ago

Maybe better to skip this release?

I wouldn't mind taking it to a vote. If we are in the middle of a bunch of stuff not doing one quarterly release is not a bad thing. Though it would also be very nice to expose @andrjohns 's new scalar binary operations so I'm very 50/50 (though they are exposed if people are using cmdstan develop). It feels like we set the release cycle quarterly because we were bad at actually putting down a date for the release. Skipping a release seems find though it does teeter towards "Well we will release when this one feature is done", which is why we made the release's quarterly lol. If we put it to a vote I'd phrase it as "We will cancel one quarterly release and next release can not be rejected by a vote".

Would a PR merging master to develop after the actual release be considered clean?

It wouldn’t automatically merge cleanly, but you could resolve any conflicts by hand. Depends on what’s happened in develop since master was pulled off.

I think what Eigen does is rebase branches before merging where they seperate out the bug fix and the feature. Then you can just cherry pick from each. Rebasing is very annoying tho

bbbales2 commented 4 years ago

I want to release! I like the regular releases. We must beckon in the fall season at Stan!

Now that I think about it, we should do 2.26 two months from now so Stan has a Christmas release and then we can be properly aligned with the seasons.

Re: Master as release candidate. Sure, or a release_2.25 branch, whatever makes life easier!

rok-cesnovar commented 4 years ago

I also want to do the release.

Other than the vectorized binary operations, OpenCL GLM support for x and y being parameters and a few other things this is more of a transitional release. Its worth it to only release for that and if there are bugs that occurred during the backend refactor we will get feedback/bug reports earlier which is nice.

Its probably to quick to make a decision now for this release, I started thinking about this too late. The clean git history is important.

SteveBronder commented 4 years ago

That was a quick vote then let's do it!

bbbales2 commented 4 years ago

this is more of a transitional release

I guess it is transitional, but I wouldn't underestimate the changes here -- all of reverse mode is changing! New autodiff for the distributions, new var type, new autodiff for basically everything that had it previously (scroll through the benchmarks -- though I'm missing a few functions and need to update it again: https://bbbales2.shinyapps.io/math-shiny-perf/).

That's gonna effect everyone. Hopefully models just go faster (the benchmarks look good). It'll be good to see how the cookie crumbles on it though.

rok-cesnovar commented 4 years ago

There is a ton of great stuff. I meant more in the sense that none of these long-term project are quite done yet.

The revamped distributions and generalization are still missing a few functions, though that is really really close. 2.26 should have all of them plus returning expressions. The var_value stuff will not land in 2.25 (maybe a function or 2 if i can get the stanc3 parts in, I am a bit scared to get stuff in the last week though) but will be in Math 3.4 that is true. The new autodiff with reverse callback is slick a hell but also not there for all functions just yet. But will be for 2.26 (or at least closer). That is what I meant by transitional.

This will all be amazing, I am not underestimating that! Please dont get me wrong, I am very hyped for all of these fun stuff.

rok-cesnovar commented 4 years ago

Basically, by transitional I meant that we cant release and say "Stan Math 3.4: var matrices reloaded", "Stan Math 3.4: expression expression expressions" or "Stan Math 3.4: calling back smoother" just quite yet, but we (probably) will for 2.26.

seantalts commented 4 years ago

How much of the reverse mode et al will be in this release for Stan users? Just curious as a Stan user myself these days, haha. Very exciting. Also if there's something I can try to hammer out on the compiler before the freeze that would help, let me know.

The clean git history is important.

I didn't mean to suggest the git history would look worse afterwards if we did another way - I think it'll be fine. I know because I've used something similar to the model you're talking about before at a few companies I've worked at.

My only argument for the current feature freeze system is that freezing the branch that everyone develops from is a cheap way to get some additional testing before it goes out to users. We went this route because when we introduced the release deadlines, folks understandably started submitting large chunks of work hours or minutes beforehand, and our automated testing coverage isn't 100% and we had some bugs slip into the release. There are probably multiple solutions to this problem, feature freezes just seemed like an easy cheap one at the time and I'm not married to it forever.

I think the "git flow" model where you tag master and build bug-fixes on master and then backport to develop is really excellent for companies with 1. great automated testing, 2. development staff on-call if a bug is discovered and 3. the ability to continuously release, usually on the web. Our situation releasing desktop software looks pretty different to me.

bbbales2 commented 4 years ago

I meant more in the sense that none of these long-term project are quite done yet

Yeah, but I've been surprised by how much stuff has changed just with the mat<var> cleanup. The thing var_value was going to do that I was most excited about was fix matrix vector multplies and make vectorization worth it.

Unexpectedly (for me) the matrix vector multiplies are way more efficient with this (like weirdly faster). Our implementation must have been doing something wrong that got fixed in the cleanup since the cleaned up version is doing all the same stuff algorithmically. It looks like the vectorized stuff is a tad bit faster too (https://github.com/stan-dev/math/pull/2121) (edit: actually, I don't think "tad bit" is appropriate here -- speeding up stuff like this is hard and it is no questions absolutely faster).

Also the stuff Tadej did with the distributions is pretty radical. I need to update the benchmarks to all he's done, but if anyone is doing gumbels, that one is nearly 3x faster for big N.

but also not there for all functions just yet

If I missed anything it was a mistake on my part: https://github.com/stan-dev/math/pull/2019 vrrm vrrrm gotta go get that green lighted

we cant release and say

I'm stuck on the var<mat> -> varmat -> varmint connection. It should be "2.26, With Varmint Tech" (and then we have a drawing of a rat with a calculator or something).

bbbales2 commented 4 years ago

"Introducing varmint tech in 2.26"

varmint

rok-cesnovar commented 4 years ago

If I missed anything it was a mistake on my part: #2019 vrrm vrrrm gotta go get that green lighted

Oh, is that PR going to move everything to reverse_pass_callback? I did not know that! If that gets in for the release that is very nice!

I'm stuck on the var -> varmat -> varmint connection. It should be "2.26, With Varmint Tech" (and then we have a drawing of a rat with a calculator or something).

:smiley:

Also the stuff Tadej did with the distributions is pretty radical.

Oh yes, those are very nice.

How much of the reverse mode et al will be in this release for Stan users?

As far as I understand (but please correct me Ben, Steve or Tadej):

move to reverse_pass_callback: Based on what @bbbales2 is saying, its done or will be done once #2019 gets in. As Tadej says, the reverse_pass_callback reduces the lines of code, reducing the chance of doing something suboptimal :) I like that quote.
var<mat>: merged in Math for multiply, elt_multiply, softmax and dot_product. Missing stanc3 side of things*.
expressions for /prim: The list of functions that do not accept expressions is shrinking daily. Once all the open PRs are merged, we will be down to:
```
algebra_solver, algebra_solver_newton
binomial
block
columns_dot_product, columns_dot_self
csr_extract_u, csr_extract_v, csr_extract_w
csr_matrix_times_vector, csr_to_dense_matrix
dirichlet
distance
dot_product
if_else
lambert_w0, lambert_w1
log_determinant
map_rect
matrix_exp_multiply
multiply_lower_tri_self_transpose
rows_dot_product
scale_matrix_exp_multiply
sd
uniform
von_mises
weibull
wiener
wishart
```
All the generalized distributions were optimized and now use expressions internally which is a huge win. The list of Stan signatures that are not tested for expressions is here: https://github.com/stan-dev/math/blob/develop/test/expressions/stan_math_sigs_exceptions.expected
OpenCL var<matrix_cl> is supported for GLMs, multiply and sum, with cholesky decompose PR open. stanc3 GLM support for this is already in stanc3. A PR for the rest and for combining them is still in the works (mostly combining them). I am hoping to maybe get it ready by Thursday, but am a bit scared of rushing it. Will see.

*I started working on that but the way it works right now we would have to check whether a given matrix is ever indexed. Since that requirement is going away in the next few weeks and the low number of functions currently supported (causing us to move var<mat> to mat<var> or back too often) that will wait for the next release I guess.

rok-cesnovar commented 4 years ago

Also, in the 70 days since the last release there were 86 pull request merged. Awesome work everyone :)

{'t4c1': 46, 'rok-cesnovar': 11, 'SteveBronder': 9, 'andrjohns': 7, 'bbbales2': 6, 'syclik': 4, 'richardfergie': 1, 'serban-nicusor-toptal': 1, 'charlesm93': 1}

wds15 commented 4 years ago

Uh... I just looked into basic reverse mode code... and it looks way different! Need to get my eyes used to this. Looks like a ton of work!

SteveBronder commented 4 years ago

Uh... I just looked into basic reverse mode code... and it looks way different! Need to get my eyes used to this. Looks like a ton of work!

Yup! Eod it will be much prettier imo. I really like how revers_pass_callback() separates out everything. The pattern feels much more intuitive to me

rok-cesnovar commented 4 years ago

A “new autodiff for dummies” would be nice after the dust settles on the major changes. This much simpler to follow for me but there are a few important details where some common recipes would be nice to have.

*by dummies I mean dummies like me.

syclik commented 4 years ago

And me.

On Tue, Oct 6, 2020 at 3:34 PM Rok Češnovar notifications@github.com wrote:

A “new autodiff for dummies” would be nice after the dust settles on the major changes. This much simpler to follow for me but there are a few important details where some common recipes would be nice to have.

*by dummies I mean dummies like me.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/stan-dev/math/issues/2128#issuecomment-704507588, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADH6F6XM4NKUBPW5572GR3SJNWN5ANCNFSM4SEMLRFA .

SteveBronder commented 4 years ago

I agree with this a lot I'll try to cook something up this week

rok-cesnovar commented 4 years ago

Are there any other PRs that must be merged before end of Monday besides https://github.com/stan-dev/cmdstan/pull/926? @SteveBronder do you need any help with that one?

bbbales2 commented 4 years ago

Address sanitizer #2146 (Edit: Done) A few optimized functions here #2106 ~Slicing functions in Stan https://github.com/stan-dev/stan/pull/2962~ (Edit: pushing this to the next release: https://github.com/stan-dev/stan/pull/2962#issuecomment-707206610)

rok-cesnovar commented 4 years ago

There were all together 6 non-CI PRs in Stan and Cmdstan so I am just publishing those release notes here. Not sure its worth opening separate release issues for the two repositories.

For stan-dev/stan:

added a const accessor to _z in base_hmc.hpp
bugfixed validate_dims in array_var_context
cleaned up includes of Eigen
~(@SteveBronder or @bbbales2) TODO: please edit-in the the slicing functions release note when/if that gets merged before the freeze~ (we're putting this off: https://github.com/stan-dev/stan/pull/2962#issuecomment-707206610)

For stan-dev/cmdstan:

enabled downloading and using a specific stanc3 version
fixed build for macOS Catalina (issues with building the Boost library)
added the output sig_figs argument to select the number of significant figures in the output CSV
(@SteveBronder) TODO: edit-in link-time optimization release note when that gets merged

For stan-dev/stanc3 see https://github.com/stan-dev/stanc3/issues/719

There were 13 code contributors across the four repositories in this release cycle : @andrjohns @bbbales2 @charlesm93 @kedartal @nhuurre @richardfergie @rok-cesnovar @rybern @seantalts @serban-nicusor-toptal @SteveBronder @syclik @t4c1 @wds15

Thanks to all. A special shout out to Richard for the first contribution to the Stan project!

The plan is still to have the release candidate ready for end of day today, but we can't release until https://github.com/stan-dev/cmdstan/pull/926 is merged as we will otherwise have a performance regression.

wds15 commented 4 years ago

Are we going to tag the branches as usual?

rok-cesnovar commented 4 years ago

Yes, freeze as usual.

rok-cesnovar commented 4 years ago

@serban-nicusor-toptal the current develop of Math is what will be the Math RC. This time around we will also do Math and Stan RCs.

Once this develop test pass and that that propagates to Stan and cmdstan we can also do those RC releases (hopefully that will be this evening). For stanc3 the nightly build = RC so no additional work needed there. Let me know if you need any help from me.

The feature freeze is otherwise now in effect and we should not merge non-bugfix PRs to develop until after the release.

rok-cesnovar commented 4 years ago

Math release notes draft:

The raw version can be seen here: https://github.com/stan-dev/math/actions/runs/304128001 I made a couple of edits and clustered them a b it.

Generalize signatures to accept general Eigen expressions (#1844, #1931, #1944, #1945, #1944, #2022, #2023, #2028, #2029, #2056, #2058, #2061, #2062, #2066, #2068, #2069, #2070, #2072, #2073, #2074, #2075, #2081, #2082, #2087, #2088, #2089, #2104, #2109, #2110, #2118, #2119, #2134, #2137, #2140)
Add templates to var and vari so that autodiff can use multiple floating point types.(#1915)
Made binomial_lpmf work more reliably when the probability parameter is 0.0 or 1.0(#1988)
Fixed requires docs(#1990)
fixed doxygen documentation of hmm_marginal(#1994)
Vectorised binary scalar functions (#1987, #2002, #2005)
Added reverse_pass_callback, a simpler and faster alternative to adj_jac_apply. dot_product implementation that accepts pointers has been removed.(#2011)
Added missing implementations for the unary plus.(#2032)
Reduced some duplicate ODE test code(#2039)
Fix problems with higher order gradients in probability test framework(#2042)
forward_as requires types match exactly -- not just be convertible(#2054)
C0 in gaussian_dlm_obs_lpdf and gaussian_dlp_obs_rng can now be positive semidefinite(#2143)
Made behaviour of fmin and fmax with equal inputs equivalent across reverse- and forward-mode(#2063)
Cleaned up the use of <Eigen/*> outside the main Eigen header file.(#2080)
fixed numerical issue with quad_form_sym.(#2096)
Added function for creating an ordered integer sequence(#2108)
The states returned by hmm_hidden_state_rng now live on {1, 2, ..., K}, rather than {0, 1, ..., K - 1}.(#2113)
Removed a duplicated check function.(#2126)

var_value:

Allowed vari to hold an Eigen type(#1952)
operands_and_partials now supports var_value operands.(#1970)
Added views for var_value class(#2024)
Added functions for conversion between var_value and Eigen matrix of vars and a metaprogram that determines return type of a function that accepts a mixture of var_values and Eigen matrices and propagates var_value if present.(#2047)
Adds softmax function for ˙var_value` matrices and testing scheme for checking array of structs vs struct of arrays matrix var implimentation.(#2050)
Adds views to vari_value<Matrix> types that allow for compound slicing expressions.(#2064)
Enabled assigning to blocks and other views into var_value.(#2065)
Adds multiplication function for var matrices and a test suite for checking the correctness of binary functions that accept var matrices as an input.(#2091)
Add var and var matrix specializtion elementwise product.(#2121)

Testing:

Updated test values for pow() to fixed false positives in the testing framework(#2123)
Upgraded Google Test to v1.10.x.(#1995)
Added Google Benchmark as a test dependency.(#1995)
Added testing framework for checking that functions exposed to stan language accept Eigen Expressions.(#1980, #2027)
Removed unit tests for reduce_sum threading that caused false positives.(#2033)
Added more tests for vectorized probability functions(#2085)
Added makefile variables that will be used for optimizations in the upstream interfaces (#2020)
Jumbo tests are used for all test/unit/math unit tests.(#2057)
Added address sanitizer to continuous integration.(#2146)

Building:

Removed the use of the GNU cut utility.(#2126)
Improved error messaging on Windows if the C++ toolchain was installed in a folder with spaces.(#2009)
Added flags to suppress warnings when building TBB.(#1993)
Switched /bin/bash with /usr/bin/env bash in the makefiles.(#1998)

OpenCL backend:

Added support for elementwise checks to kernel generator(#1977)
Removed matrix_cl<var> specialization.(#2021)
OpenCL implementation normal_id_glm_lpdf can also be used when derivatives of x or y are needed.(#2034)
OpenCL implementations of bernoulli_logit_glm_lpmf and poisson_log_glm_lpmf can also be used when derivatives of x are needed.(#2035)
Added index operations to kernel generator.(#2051)
Implemented indexing for kernel generator expressions.(#2052)
OpenCL implementations of neg_binomial_2_log_glm_lpmf, ordered_logistic_glm_lpmf and categorical_logit_glm_lpmf can also be used when derivatives of x are needed.(#2055)
Added an option to select the OpenCL device to use at runtime.(#2067)
Simplified tests of GPU GLMs by moving repeated code into a common utility header.(#2097)
Added OpenCL reverse mode implementations of matrix multiply and sum.(#2099)
vari_base can now be constructed with matrix_cl value and adjoint. Copying between host and device is possible for vars containing Eigen::Matrix/matrix_cl. Adjoints are propagated over these copies.(#1967)

99 PRs merged alltogether

@SteveBronder volunteered for the "narrative" version that we did last time, to highlight a few things we want the users to definitely not miss.

bbbales2 commented 4 years ago

Functions check_not_nan, check_nonnegative

2007 got reverted, so I'm going to edit those out

rok-cesnovar commented 4 years ago

Oh right, thanks. In the future when reverting PRs we should go and remove the release notes sections from the PRs we are reverting to avoid this.

serban-nicusor-toptal commented 4 years ago

Hey, I've done the RCs for math, stan and cmdstan ( including stanc3 nightly binaries ). You can find them here: math, stan. cmdstan.

I did not include the release notes in github so we don't have them again on the release next week.

rok-cesnovar commented 4 years ago

@avehtari raised a good point that the freeze should last for a week after an annoucement with the feature rundown is posted. That would mean the freeze would be prolonged for 2 days.

Does anyone have strong objections to that? Seems like a reasonable suggestion to me.

SteveBronder commented 4 years ago

Hey yeah that sounds reasonable I'll do the write up tonight

SteveBronder commented 4 years ago

Rok when you get a minute can you make the release notes bullet points into a table and have the script add the name of the person who opened the PR?

rok-cesnovar commented 4 years ago

Sure. Do we need this for the RC annoucement? We usually add this to the "official" release notes.

SteveBronder commented 4 years ago

Oh yeah as long as we have it for the official release notes that's fine

SteveBronder commented 4 years ago

Apologies I'm turning this around a bit late. Feel free to remove the silly joke at the end (and change whatever else you like)

https://gist.github.com/SteveBronder/0f6f39e27b954d6002004768b4234e7a

rok-cesnovar commented 4 years ago

Thanks. Will take a look and then you just post it whenever you wakeup.

wds15 commented 4 years ago

BTW... I think we should announce the RC and then wait for a week as suggested by @avehtari ... so @SteveBronder maybe note that in the RC post would be my suggestion (release is scheduled for 22nd Oct).

rok-cesnovar commented 4 years ago

@SteveBronder I can't really comment in a nice way on a gist so I made some edits: https://gist.github.com/SteveBronder/0f6f39e27b954d6002004768b4234e7a#gistcomment-3490406 Also included a note on the release schedule.

Please read it through and then feel free to make an announcement on the forums.

serban-nicusor-toptal commented 4 years ago

Hey, everything ready for the release today on 22 Oct ?

t4c1 commented 4 years ago

We should wait for one last minute bugfix in https://github.com/stan-dev/math/pull/2156.

rok-cesnovar commented 4 years ago

There were a few bugs found. In addition to what @t4c1 says, there is also https://github.com/stan-dev/stanc3/pull/728 and a really small cmdstan makefile variable renaming PR that will be opened in a few minutes.

All of these are small fixes, so we can probably still release today, but it may have to be tomorrow.

rok-cesnovar commented 4 years ago

@serban-nicusor-toptal All the fixes are now in. Once the develop tests pass and the submodule changes propagate up to cmdstan everything is ready for the release that should be done on Monday.

serban-nicusor-toptal commented 4 years ago

Hey, I hope everything is ready for the release. I will wait 2 hours and if there are no objections will do the release.

rok-cesnovar commented 4 years ago

Yes, everything is ready. Latest develop states of Math/Stan/Cmdstan and the latest nightly build of stanc3 are all good.

rok-cesnovar commented 4 years ago

One more time so you have everything here.

Math release notes: https://github.com/stan-dev/math/issues/2128#issuecomment-707677621 Stan release notes:

added a const accessor to _z in base_hmc.hpp
bugfixed validate_dims in array_var_context
cleaned up includes of Eigen

Cmdstan release notes:

enabled downloading and using a specific stanc3 version
fixed build for macOS Catalina (issues with building the Boost library)
added the output sig_figs argument to select the number of significant figures in the output CSV
added STAN_CPP_OPTIMS makefile flag to turn on link-time optimizations and other C++ compiler optimizations

Stanc3 release notes: https://github.com/stan-dev/stanc3/issues/719#issue-719151524

Thanks Nic!

serban-nicusor-toptal commented 4 years ago

Thanks @rok-cesnovar I'll start with the release now.

serban-nicusor-toptal commented 4 years ago

Release for math v3.4.0, stan v2.25.0, cmdstan v2.25.0, stanc3 v2.25.0 are now out! If anyone has a few minutes to take a look over so we're double-sure everything is good that would be awesome, thanks!

I'm now creating PRs for docs.

wds15 commented 4 years ago

The cmdstan tar.gz file posted unpacks to the folder cmdstan... so the version is missing. Can u add that please?

stan-dev / math

Release of Stan Math 3.4 #2128

2007 got reverted, so I'm going to edit those out