Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qiskit release cycle and versioning #34

Merged
merged 28 commits into from
Mar 5, 2024
Merged

Conversation

1ucian0
Copy link
Member

@1ucian0 1ucian0 commented Apr 26, 2023

A new versioning schema for a more clear stability cycle. SemVer with CalVer elements.

@garrison
Copy link
Member

garrison commented May 15, 2023

My first impression is that I think that bumping the major version once a year sounds about right. If semantic versioning is followed, this means only introducing breaking changes once per year. I could see two workflows that could support this.

Option 1:

  • Both major versions and minor versions are branched from main shortly before release.
  • New features can be merged anytime throughout the year, which then appear in the next minor release. However, breaking changes can only be merged in a 2-3 month window that occurs once per year. In the worst case, breaking changes must wait up to ~10 months before they can merge, ~12 months before they can be released.

Option 2:

  • Major versions are branched from main shortly before release. Minor releases are then branched from the most recent major release branch shortly before that minor release.
  • Breaking changes can be merged to main at any time throughout the year.
  • All non-breaking, new features need to be backported to the most recent major release branch right after the merge to main.

I prefer Option 1, as it means fewer branches and directs human effort and eyeballs toward the next minor release. Breaking changes can be difficult and annoying to backport, too. But I fear that there will be pressure to hold up each major release for "just one more breaking change" with a release cycle so long. Especially in that case, the deadline for having a PR ready must be well before the major version's release date. Just as Linus Torvalds hates when merge requests come in at the end of the rc1 window, I could see this leading to some significant pressure on the Qiskit maintainers.

Having thought through this more thoroughly, would we be better off to start with a 6-month cycle for a bit? One major release, then one minor release 3 months later, then back to major again 3 months after that, etc. Many, many open source projects -- from desktop environments to entire distributions -- have found 6 months to be the correct cadence.

A few remaining thoughts/questions:

  • DeprecationWarnings should not be introduced in a minor release if semantic versioning is being followed. I think it would be fine for a minor version to introduce a PendingDeprecationWarning, though, which would then become a DeprecationWarning in the following major release. EDIT: I was wrong about this point; see Jake's post immediately after this one.
  • I don't fully understand the bars in the mermaid plots. Do they represent the fraction of time that a branch should expect to receive stable backports?

@jakelishman
Copy link
Member

I'm with Jim above. I think his option 1 is probably the right way for us to try, at first at least - I don't think we have sufficient manpower or dev-team experience to attempt to manage development of two versions simultaneously (which is kind of what option 2 devolves to without a lot of effort). I'd also be happy to try a six-month cadence initially (also content to stick with yearly), though in this scenario we'd only have one minor release between majors, so I'm not certain if it'd actually buy us too much.

Just to speak to the penultimate point: the formal semantic versioning spec allows deprecations to be added in minor releases (link to rule, link to faq entry). To keep the spirit of our current deprecation policy, I think we should still maintain the rule "deprecation warnings can only occur 1+ minor releases after the replacement functionality is added", and potentially strengthen the removal rule to "deprecation warnings must be emitted for a complete major version" (i.e. 1+ year).

####-release_cycle.md Outdated Show resolved Hide resolved
### Deprecations

* deprecation warnings can only occur 1+ minor releases after the replacement functionality is added
* deprecation warnings must be emitted for a complete major version" (i.e. 1+ year)
Copy link
Member Author

@1ucian0 1ucian0 May 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added from #34 (comment) @jakelishman

I would like to offer an alternative as this sounds like a lot of time to me. Introducing a deprecation warning in X.Y and not being able to remove it until X+2.0 is a huge time period of dangling code. If we keep X.3 support for 6 extra month, that should be enough to remove it in X+1.0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the trouble with this approach is that it assumes that every user will upgrade through every minor version, so will see deprecations introduced in minor releases. To me, introducing major versions means that we expect that the required upgrade path is only at the granularity of "through each major version", and so my form is what guarantees that a user will see every deprecation warning.

Copy link
Member

@jakelishman jakelishman May 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, currently our upgrade strategy is that you can't expect to go from Terra 0.22 to 0.24 without something breaking without warning - you must go via 0.23 to ensure you've seen all warnings and had the opportunity to smoothly upgrade, and the deprecation policy also includes elements that require that there's a smooth no-error path you can take for a downstream library to support two consecutive minors, which is necessary for library authors to avoid needing precise latest-version-only pins.

I would foresee that if we add major versioning, the story ought to be "you must upgrade through each major, but you needn't install each minor", and then to keep the same spirit as our current deprecation policy, you need the deprecation warnings through an entire major release cycle.

I should be clear that what I said above is stricter than the official semantic-versioning story (which only requires a single minor release for a deprecation).

Copy link
Member

@garrison garrison May 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it assumes that every user will upgrade through every minor version, so will see deprecations introduced in minor releases.

Actually, the necessary assumption is weaker: before upgrading to a new major version, the user would only need to use the final minor version of the previous release, because there, all deprecation warnings necessary for upgrading will be visible and can be fixed.

Julia is an example that illustrates both the benefits and drawbacks of this approach. Julia 0.7.0 and 1.0.0 were released within ~48 hours of each other. The only difference between them is that 0.7.0 contained compatibility (and deprecation warnings) for code that worked under 0.6, while 1.0 removed all such support. If there is ever a Julia 2.0, I expect the same thing to happen: users will be expected to use the "final" 1.X release and fix all deprecation warnings before trying the code under Julia 2.0.

The downside to this approach was that, in practice, people not intimately familiar with the release plan upgraded directly from 0.6.x to 1.0, and were met with a poor user experience: tons of code that worked on the previous release simply failed. If the goal is to get users to use a certain release before upgrading, it really does need to be emphasized in all the obvious locations, and then some.

Copy link
Member Author

@1ucian0 1ucian0 May 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

people not intimately familiar with the release plan upgraded directly from 0.6.x to 1.0, and were met with a poor user experience: tons of code that worked on the previous release simply failed.

I see this as expected, as a major upgrade.

I see the trouble with this approach is that it assumes that every user will upgrade through every minor version

Indeed. This is an assumption that needs to be explicit. However, it is not more obscure than the current one.
Downstream library should depend on the fairly standard ~=X.Y or ==X.*. With the extended period for X.4.* receiving critical bug fix support and not introducing new deprecations, pinning the previous major is always an option and gives a good period of time to migrate.

Copy link
Member

@jakelishman jakelishman May 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My issue is that adding major versions into the mix does change the "obscurity" of the policy. I expect that code I write for one version of a library with no warnings will work in the next version of that library (now maybe with warnings), at whatever granularity that library does it major releases. Once we have majors, I see them as defining the required-upgrade granularity, as opposed to our current system where minors are that top level. That's not a requirement of semantic versioning as a spec, but that's because that spec is only for associating the scope of changes with the version number, not for any of the development/upgrade strategies that go with it.

Requiring that the 2->3 upgrade goes via 2.last and heavily publicising that is one way of dealing with my concern, but I think we've seen that we won't ever reach all our users, and I'd argue that this is a much more "obscure" requirement. People expect instability when something's versioned 0.x, but there's more expectation of stability when it's 1.x+.

edit: I think this is mostly opinion-based, so likely a resolution to this will come from a vote rather than one of us actually convincing the other.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@garrison I actually really liked the Julia approach of having a final minor version to introduce all the deprecations prior to the major version bump. It provided a concrete path to follow as a developer to get to working code despite introducing a lot of change in 1.0.

In general, I think relying upon the final minor version as a necessary step to expose all the deprecations is a very teachable system.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The notion of "final minor release" was introduced in 3ec912b

@1ucian0
Copy link
Member Author

1ucian0 commented May 19, 2023

@garrison said:

I don't fully understand the bars in the mermaid plots. Do they represent the fraction of time that a branch should expect to receive stable backports?

Yes, the plot represent how long version has support. X.1 is supported until X.2 is released, for example.

@blakejohnson
Copy link
Contributor

What if instead of committing to releasing a new major version each year, we simply commit to updating which version is considered "stable"? That decouples semvar signaling of version compatibility from a statement about stability / support.

@mtreinish
Copy link
Member

What if instead of committing to releasing a new major version each year, we simply commit to updating which version is considered "stable"? That decouples semvar signaling of version compatibility from a statement about stability / support.

I think that's the crux of the discussion here. What Luciano and I discussed for this RFC is basically doing calver but with semver like naming. I definitely think a more traditional semver approach where we don't commit to 2.0 at a specific time and do it when we're ready has a lot of merit. In general if we go with a semver naming I would actually be more in favor of decoupling the passage of time from when we bump the major version number. However, the reason we opted for putting one release a year in the RFC was because there is a hesitancy to commit to not making any breaking API changes for > 1 year (or conversely breaking more than once a year and doing multiple major versions in a year) especially given the pace of Qiskit and quantum computing development as a whole.

The two paths forward I think are we can either go fully to calver and just use date based naming so use 24.0.0 for the first release next year and commit to stability/no breaking API changes for each calendar year. This ends up leaving the rfc pretty similar to what's there now (just adding 23.0.0 to all the version strings), with an open question on how we handle deprecations/notifying users of breaking changes between each year. Also how long we continue to support a particular year's releases.

The other option is use semver naming and semantics and bump the major version when we're ready to break API compatibility, not at a fixed interval and message around the stability and upgrade path between version numbers. The caveat here is we probably should put a minimum duration between major versions (similar to how the current deprecation policy sets a minimum of 3 months of support), so say at least 1 year between major versions so that we're actually committing to stability for some time, and for example not just going to release 4.0.0 in January and then 5.0.0 in April of the same year.

@1ucian0
Copy link
Member Author

1ucian0 commented Jun 5, 2023

Option 1:

Both major versions and minor versions are branched from main shortly before release.
New features can be merged anytime throughout the year, which then appear in the next minor release. However, breaking changes can only be merged in a 2-3 month window that occurs once per year. In the worst case, breaking changes must wait up to ~10 months before they can merge, ~12 months before they can be released.

I'm with Jim above. I think his option 1 is probably the right way for us to try, at first at least

I agree with option 1 too. I added it in b9fdb99 and e401ee3

@wshanks
Copy link

wshanks commented Sep 16, 2023

A couple things I think it would be good to include (though it seems like the draft might change a lot based on the comments above):

  1. The git graph for the proposed new branching model on shows features and bugfixes. Where do the breaking changes happen? Is it that all breaking changes are removals guarded by a deprecation warning and then removed the ready_for_plus_one commit? There could be behavioral changes but maybe those would be forced to be an addition of a new name plus a removal of an old one, rather than a change in behavior of one function or method (not sure if that is always possible). Partly I am wondering how one tests the next version of qiskit without an extra branch with the breaking changes. Maybe they get applied after X.3 and then can be tested between then and X+1.0.
  2. There is open discussion about the support cycle above, so the current proposal may not be followed any way, but I think it might be worth including some guidance for downstream projects. For example, currently, the IBM client packages qiskit-ibm-provider and qiskit-ibm-runtime tend to bump the minimum version of qiskit(-terra) they depend on soon after there is a new qiskit release. Since these clients also include changes necessary to keep working with the IBM Quantum API, it is not viable to hold back on an old version of the client and so anyone working with IBM systems has to upgrade qiskit promptly as part of the client upgrade. So at least for those working with IBM systems, there would not be much value to providing a long support cycle for X.3 after X+1.0 is released if things keep working as they have been.

mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Nov 6, 2023
This commit document expands the versioning, deprecation, and stability
polices to document the policy around major versions. The previous
policies were all written without any consideration for major
versioning. This outlines the minimum support windows for major versions
and how deprecations and breaking changes will work with regards to
versioning. It also explicitly documents that Qiskit is using semantic
versioning. This is a actually a change from historical releases prior
to Qiskit 0.44.0 where we explicitly did not follow semantic versioning
as the version number was based on the qiskit elements.

This is an expanded document outlining the procedures decided on in
the discussion around Qiskit/RFCs#34.
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Nov 6, 2023
This commit document expands the versioning, deprecation, and stability
polices to document the policy around major versions. The previous
policies were all written without any consideration for major
versioning. This outlines the minimum support windows for major versions
and how deprecations and breaking changes will work with regards to
versioning. It also explicitly documents that Qiskit is using semantic
versioning. This is a actually a change from historical releases prior
to Qiskit 0.44.0 where we explicitly did not follow semantic versioning
as the version number was based on the qiskit elements.

This is an expanded document outlining the procedures decided on in
the discussion around Qiskit/RFCs#34.
@1ucian0 1ucian0 marked this pull request as ready for review December 11, 2023 14:44
@1ucian0
Copy link
Member Author

1ucian0 commented Dec 11, 2023

I think all your comments are addressed. Have a look!

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some small things inline. I think we can merge after those corrections.

####-release_cycle.md Outdated Show resolved Hide resolved
####-release_cycle.md Outdated Show resolved Hide resolved
####-release_cycle.md Outdated Show resolved Hide resolved
####-release_cycle.md Outdated Show resolved Hide resolved
####-release_cycle.md Outdated Show resolved Hide resolved
####-release_cycle.md Outdated Show resolved Hide resolved
####-release_cycle.md Outdated Show resolved Hide resolved
####-release_cycle.md Outdated Show resolved Hide resolved
####-release_cycle.md Outdated Show resolved Hide resolved
1ucian0 and others added 8 commits March 1, 2024 12:52
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
####-release_cycle.md Outdated Show resolved Hide resolved
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@1ucian0 1ucian0 merged commit b92632e into Qiskit:master Mar 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC proposal New RFC is proposed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants