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