Stan 2.21.0 released

Announcements
1

Major changes include a new threading library dependency that has shown great performance improvements for map_rect models with threading but also introduces a new licensing restriction - we cannot be compiled into the same binary as GPLv2 code anymore, and Math is no longer a pure header-only library (so @syclik bumped it to 3.0.0 with this release). I don’t want to restate everything from the release notes here, so please check them out for CmdStan, Stan, and Math.

We nearly released the new compiler (stanc3) in this release as well, but in the meeting Thursday we decided we wanted to set a much higher bar than we have for the current compiler and let users do some more testing before it’s in an official release. It’ll be back on CmdStan develop as soon as a couple of known bugs are squashed (hopefully next week), and we’re also planning to add some features to enable more users to test from CmdStanR, CmdStanPy, and RStan in their next releases.

Thanks to everyone who contributed! Lots of heroic pushes before the feature freeze at the end there, which really made me smile. If you have thoughts about the feature freeze we’re going to discuss how it went at that thread.

9 Likes
2

Thanks, @seantalts.

I just want to be clear: the bump in version number happened because we were following semantic versioning rules and the Math devs recognize that there are non-backwards-compatible changes in this version.

3

Ah, okay. Do you wanna paste a link to that discussion?

4

Yup. It’s summarized here. Monthly Math development meeting: 10/17/2019, 10 am EDT

5

Some of the discussion also happened here: TBB and 2.21

6

I missed the first half of the math meeting but assuming the discussion is mostly captured online for this decision on that thread, I think my original comment was correct that @syclik made the decision to bump it to 3.0.0. The other folks on that thread disagreed with your reasoning. Which is fine, you’re the tech lead for Math and can pull rank. But it’s good to acknowledge when executive decisions are made.

7

Thanks for brining up your concerns. There really wasn’t a decision made at the meeting and I didn’t pull rank or force anything.

The decision points were:

  • to use semantic versioning, which happened ages ago on the forums
  • to create backwards incompatible code; this was discussed on the PR
  • to include backwards incompatible code into the Math library; this action already happened.

At the meeting, I made sure I understood the impact of the code and following the rules we laid out for ourselves. I consider a decision to have been made if we decided against our own policies. In this case, we came to an understanding and were just following through with what are our own procedures. Once it was clear that the code indeed broke backwards compatibility, it was clear that we were to bump major version. At the end of the discussion, we were all clear on the same points. (And yes, it was split across different threads.)

Does that explanation help?

8

You’re right that we’ve been using semantic versioning for ages and that the code was already on math. But there is interpretation involved in what the API is (even given the wiki page) and thus what is considered backwards-incompatible. There are logical, consistent arguments on both sides; you pushed for your interpretation while others disagreed and it reads like you ultimately made the decision. Which again, is all fine, but I’m not sure why you don’t want to take credit for that decision… I think we ultimately need tech leads (or soon, chairs I hope) to help us make these kinds of decisions, but this kind of TWG hierarchy won’t work if it’s not clear when a tech lead uses their authority as tech lead as that removes responsibility, positive or negative, from the position. I think this is ultimately a pretty inconsequential decision but it’s worth being accurate about the process.

9

I completely agree! There’s definitely some miscommunication here and I think we can sort it out. We’re on the same page with the ideas here.

I’m happy to own any decision I make. (Honestly. For better or worse, I’ll own up to my decisions.) I agree that this is also inconsequential, but we should be accurate about what happened. And agreed that it’s common knowledge that we’ve been using semantic versioning on Math.

Yes, there is an interpretation involved with what is the API, but online and at the meeting, we came to the conclusion that this release is not backwards compatible with how we’ve come to understand it (and loosely defined it). The original PR submitter, @wds15, thought it was backwards-incompatible from the point of view of Math (but not from the point of view of interfaces like CmdStan and RStan). @rok_cesnovar, the reviewer, also knew that this was not backwards compatible, but wasn’t as concerned with the version numbering that having backwards-incompatible code introduces.

The decision to include the code was a sound one and made through our normal PR process. There was an issue, a submitter put together a PR, a reviewer reviewed it, and it actually went through some revisions. It was finally accepted, without anyone rejecting it. (Yay! That’s great! It means the codebase is moving.)

The version numbering discussion had been ongoing… at the time of the prior Math dev meeting and at the start of the PR process, the proposed design was meant to be backwards compatible. By the time the PR got in, the design had changed (for the better) and it was backwards incompatible. I know the API is open to interpretation, but I think it’s pretty clear that this release isn’t a simple drop-in replacement for an older version of Math. There was a lot of work to get this working downstream.

So… the version numbering thing wasn’t so much a decision but rather asking the devs to follow the semantic versioning procedure. There was a question of whether we had enough of an argument to ignore semantic versioning for this release. And @seantalts, you were there to help us reason that it’s ok to have a separate version for Math.

The reason I didn’t think it was a decision: once a backwards incompatible change was merged into the develop branch, we just follow through with procedure laid out by semantic versioning for the next release. I just don’t see the “decision” at that point. It was made on when the PR was merged. On the PR, there was a discussion about accepting this change unconditionally, making it backwards incompatible. Not only did the implementation go this way, but there were many voices in support of it for the sake of simplicity. If that sort of change implies we have to upgrade a major version, that’s fine and that’s what happened here.

A fair question to ask is whether I brought this up as a math dev or someone with authority in Math. The answer to that is that it could probably be seen as both, but anyone could have brought this up and we should have ended with the same conclusion. We have a backward incompatible change to the API; this implies that the next version is a bump in major version. A deviation from that policy would have been a decision… if I didn’t bring it up, we probably would have made that implicit decision to deviate from semantic versioning. There wouldn’t be such a large consequence from that, but as a user of software, I do appreciate when packages indicate when I have to make changes to upgrade. To get back to the question… I was actually approaching this as a dev. I walked into the meeting asking for how this particular PR was compatible with v2.20. The answer that explained why it was viewed as backwards compatible was that it’s backwards compatible from an interface, meaning RStan code that works for v2.20 will work with the next release. After talking through it, however, we were all on the same page that for Math, the code that worked for v2.20 would no longer work with this next release. As in… if you use the Math library, it won’t compile with the next release.

I really hope that helps. If you believe that the latest release is not backwards compatible with the previous one, do you still believe that I ultimately pushed through a decision? That was definitely not the intent. I had thought that we had gotten to the point where we realized the impact of the code on our users and justifiably called the next version v3.0. (At the meeting, there wasn’t a confusion about the API boundary on this particular change… if you have working Math code that assumes header-only, it does not work with the next version. There’s not an option to have it work. It just won’t. Hence… the flurry of PRs into the downstream repos after this was merged.)

10

@seantalts, how could I have made it clearer that this just business as usual?

11

Just to be clear I was fine with this decision – the logic seemed fair enough to me. Pretty sure @wds15 was happy with it at the meeting as well. @ariddell approved here: Monthly Math development meeting: 10/17/2019, 10 am EDT - #4 by ariddell, and @rok_cesnovar approved here: TBB and 2.21 - #35 by rok_cesnovar

Part of the TBB and 2.21 thread happened before the Math meeting.

1 Like
12

Agreed, we were all fine with @syclik’s decision.

The decision was how to interpret the API w.r.t. backwards incompatibility. There is a coherent and strong argument that Math 3.0.0 is backwards compatible if you consider our API to require using our makefiles, but that wasn’t specified anywhere and there’s an equally strong argument that our API shouldn’t require that.