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

Pin upper version on PyBaMM #289

Closed
BradyPlanden opened this issue Apr 12, 2024 · 4 comments
Closed

Pin upper version on PyBaMM #289

BradyPlanden opened this issue Apr 12, 2024 · 4 comments

Comments

@BradyPlanden
Copy link
Member

BradyPlanden commented Apr 12, 2024

As discussed in #285, pinning an upper version limit on PyBaMM would ensure breaking changes are mitigated to PyBOP users. Further discussion on this topic is here:

This comment is not a review, but more of a question about our policy for dropping these versions. Is the plan to follow PyBaMM strictly and create another release for PyBOP as soon as possible after PyBaMM's own release?

There could be a case where PyBaMM drops support for Python v3.A in its new version v2X.Y, and a current PyBOP v2M.N might still support Python v3.A, which means one matrix target will start to fail, creating a signal to drop the version for us here at PyBOP. Users running PyBOP v2M.N at that time on Python v3.A will notice that the new PyBaMM v2X.Y will not be downloaded by pip because of the Python version constraint, and therefore they would be using an older version, say PyBaMM v2X.W, which is alright. However, those on newer versions of Python, say, v3.B, will not face this constraint because it will be supported by both PyBOP and PyBaMM (assuming that both packages will support an array of at least four Python versions simultaneously). Therefore v3.B will compel pip to download PyBaMM v2X.Y + PyBOP v2M.N. Here, there could be breaking changes for PyBaMM which PyBOP would not be supporting. As a result, PyBOP would need to put out a new release after v2M.N with a new identifier, say, v2M.O to support the new PyBaMM v2X.Y along with previous PyBaMM versions (but if it's not possible to support older ones, then drop an old one and support a new one at the same time) and/or drop support for Python v3.A (and add support for newer Python versions supported by PyBaMM). Therefore, I have these comments:

  1. Maybe PyBOP should consider upper-pinning PyBaMM in its dependencies (in addition to lower bounds) to avoid such behaviour
    a. There is a lot of nuance involved with upper-bound constraints: https://iscinumpy.dev/post/bound-version-constraints/
  2. The answer is also highly dependent on how well PyBaMM documents its breaking changes and features, and also on the extent of the breaking changes (it's unlikely that, say, a PyBaMM solver changes its functionality significantly or anything, but other things like parameter sets with the BPX standardisation could do so – that I do not have a lot of information about). It would also be reliant on how users are going to use PyBaMM and PyBOP together and how reliant the latter will be on the former in terms of certain functionality and offering convenient features.
  3. I suppose these issues would only be encountered on those that are running edge versions of Python (say, Python v3.A and Python v3.D), and not those intermediate ones (Python v3.B, v3.C, etc.) that will continue to stay in support for years on stretch and will be dropped only after a considerable amount of releases (i.e., not in the immediate next releases for either of PyBaMM or PyBOP).

I guess I am just concerned about the synchronicity between PyBaMM and PyBOP; there is a chance that my concerns could be misplaced, but I hope that what I am asking makes a bit of sense and is something to ponder about!

P.S. Adjusting this script to cater to new Python and PyBaMM versions or ignore specific ones is quite trivial, of course, and we don't need to worry about it.

Originally posted by @agriyakhetarpal in #285 (comment)

@agriyakhetarpal
Copy link
Member

Thanks for opening this as a new issue, @BradyPlanden. I would also point out that upper-pinning should be as liberal as possible so that it opens up space to use a reasonable number of PyBaMM versions, if upper-pinning has to be done, otherwise it can cause conflicts if PyBOP is used as a library in coordination with other packages (as a library, that is). The blog post linked above has many insights. I think Saransh would also have some more information about this (cc: @Saransh-cpp). The good thing, though, is that PyBaMM does not have any upper pins for any package and pins just one of its dependencies ([jax], which is optional), so PyBOP won't experience any issues with the PyBaMM side at least (but it could very much face problems with supporting other current dependencies besides PyBaMM).

@Saransh-cpp
Copy link

Pinning a dependency is not recommended. Pinning a particular library will make your code behave like an application, and not like an extendable library. If this repository is meant to be used as an application and users are not expected to depend on it, it should be fine to pin PyBaMM. In extreme cases, pinning a library is okay but it should be avoided.

@agriyakhetarpal
Copy link
Member

agriyakhetarpal commented Apr 14, 2024

Thanks, @Saransh-cpp. That's what I think, too. I was thinking of a very wide/forgiving upper-pin, but I think the best way forward would be to actively track changes on PyBaMM's side, through:

  1. PyBaMM's release notes (which we will be actively updating before each release: Release notes 24.5 pybamm-team/pybamm.org#113),
  2. Dynamic GHA matrices running against released PyBaMM versions nightly (already done in Create dynamic GitHub Actions matrix to test against PyBaMM #193),
  3. Testing against PyBaMM on a schedule from its develop branch (as is being done in Add workflow to test PyBaMM develop branch nightly #291 right now, which will be updated after Add CI jobs to upload nightly wheels to Anaconda pybamm-team/PyBaMM#3945)

and then PyBOP devs shall cut a release for PyBOP with updated lower bounds on PyBaMM, which will be soon after PyBaMM gets to its stable release, so that users still on older versions of PyBOP will receive compatible versions of PyBaMM from pip (and then they can upgrade their scripts and simulations at their discretion – therefore, newer PyBOP versions will create implicit upper-pins on PyBaMM for use by older PyBOP versions).

@agriyakhetarpal
Copy link
Member

I guess this should be okay to close with the other PRs being open right now, and we can evaluate what to do on a case-by-case basis.

@agriyakhetarpal agriyakhetarpal closed this as not planned Won't fix, can't repro, duplicate, stale Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants