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

Package Brian2CUDA #288

Merged
merged 10 commits into from
May 16, 2022
Merged

Package Brian2CUDA #288

merged 10 commits into from
May 16, 2022

Conversation

denisalevi
Copy link
Member

@denisalevi denisalevi commented Apr 18, 2022

I started preparing a release. This PR should take care of packaging and versioning and should be merged only once everything else is ready.

EDIT: TODO list:

  • Change package name from brian2cuda to Brian2CUDA
  • Add PyPi and TestPyPi credentials (will every tag be pushed?, was the check in this PR only a check or a try to publish already?)
  • Change github action to only push tags starting with v and change the versioneer section in setup.cfg accordingly. This allows to have other tags than release tags, while the version is still the version without v.
  • Add @mstimberg to PyPi project once we published the first release (and maybe to renamed TestPyPi if necessary?)
  • Update (Test)PyPi credentials after merging this issue to be credentials only for this repo (currently scope are all packages on my account because Brian2CUDA wasn't ever uploaded yet)
  • Do we need a MANIFEST.in? It was added during versioneer install. What is this exactly for?
  • Rename master to main (needs changes in github actions as well)
  • Update LICENCE to GPL v. 3
  • Add CITATION
  • (semi-optional) Create conda-forge package without any CUDA related package (only brian2 dependency), same as Brian2 receipe in the conda-forge feedstock, triggered by PyPi release (?)
  • (optional) Modify publish action to create github release as well. Use one of the maintained actions here. Also come up with some way to store release notes in separate files, sticked together in the docs, but also used for github releases. Similar to how numpy does it.
  • (optional) Use pyproject.toml (and setup.cfg?) instead of setup.py

I updated setup.py and uploaded a PyPi package to Test PyPi: https://test.pypi.org/project/brian2cuda/.

I tested it locally and on one of our clusters, worked without issues. Even when installing brian2 via conda first. As far as I can tell, Brian2CUDA has no issues with conda environments (we use CXX for host compilation, which uses the C++ compiler in the conda environment). And I had another look at nvcc_linux64: All it does really is to provide a wrapper around a specific nvcc version and make sure that nvcc uses CXX for host compilation (which we do anyways). It also adds cuda/include include paths to C/CPP/CXXFLAGS, but as far as I can tell, we don't need them (it works without in my test cases). I would add nvcc_linux64 as optional package to the installation instructions in case people want to choose a specific nvcc version in their conda environment. But I wouldn't install it by default. Brian2CUDA already detects the default nvcc and one can control the nvcc path via Brian2CUDA environment variables or preferences as well.
Hence, I see no problem in providing a conda package, all it needs is the correct Brian2 dependency and that's it.

@mstimberg Do you have time to support me with versioning and packaging? Few questions I had by looking into Brian's config files:

  1. On TestPyPi, the Brian2 package is called Brian2 instead of brian2. Any specific reason for that, does it make sense to do the same for Brian2CUDA?
  2. Brian2 doesn't use GitHub's release option, instead it only uses tags. I saw that the GitHub action for PyPi publishing relies on pushing tags to the repo, is that the reason? Or anything else?
  3. Could you review my setup.py if it looks alright to you?
  4. I had a look at the conda receipt and PyPi publish action. There were a few things I didn't understand, like Windows- and Cython-specific setups. If you have time, could you maybe strip both receipts down to the bare minimum (deleting everything Windows- and Cython-specific) and pushing it to this branch? I think that would save me a lot of time and should be quick for you I think?
  5. I'd also like to use versioneer, as far as I understand, I just have to copy the Python file over in this repo. Will have a look at that. I would say our first release here will be a beta release. Have to decide if we want to version that as 0.1 and upgrade to 1.0 once we think beta is over? Or go with 1.0b1 (maybe depends also on versioneer?).

@denisalevi
Copy link
Member Author

I had another look #212 and brian-team/brian2genn#101. I still think that the problems that came up there might not be relevant for Brian2CUDA (at least in my hands, I mostly had similar issues with Brian2GeNN, while Brian2CUDA mostly just worked), I would like to try to get a good conda package working. It might even be worth trying out cudatoolkit-dev to ship the entire toolchain. But I will leave that for now, first need to get some more documentation up. If I find time to look into it, that would be great. But this could also happen after the first release. For now, a pip package is working.

@mstimberg
Copy link
Member

  1. On TestPyPi, the Brian2 package is called Brian2 instead of brian2. Any specific reason for that, does it make sense to do the same for Brian2CUDA?

We call the package Brian2 in setup.py, and that's why it is called this way on PyPI and TestPyPI. There's no particular reason for this, but PEP 426 guarantees that both names are considered equivalent.

2. Brian2 doesn't use GitHub's release option, instead it only uses tags. I saw that the GitHub action for PyPi publishing relies on pushing tags to the repo, is that the reason? Or anything else?

Again, no particular reason, we always used tags. The advantage is that you simply add a tag locally and push it to trigger a release instead of using github's GUI for it. I guess one could now use GitHub's CLI as easily, but well, it's not a high priority to change it, I think. The only advantage I'm seeing is that releases are more visible on GitHub and can have release notes.

3. Could you review my setup.py if it looks alright to you?

Looks good to me (did not test anything, though).

4. I had a look at the conda receipt and PyPi publish action. There were a few things I didn't understand, like Windows- and Cython-specific setups. If you have time, could you maybe strip both receipts down to the bare minimum (deleting everything Windows- and Cython-specific) and pushing it to this branch? I think that would save me a lot of time and should be quick for you I think?

The conda recipe is out of date and we don't use it anymore (we should probably delete it...). The relevant one for conda-forge is the one here: https://github.com/conda-forge/brian2-feedstock/blob/main/recipe/meta.yaml But it's not guaranteed to correctly works like this on a local machine, it relies on the conda-forge infrastructure. I'll have a look at the PyPI action.

5. I'd also like to use versioneer, as far as I understand, I just have to copy the Python file over in this repo. Will have a look at that. I would say our first release here will be a beta release. Have to decide if we want to version that as 0.1 and upgrade to 1.0 once we think beta is over? Or go with 1.0b1 (maybe depends also on versioneer?).

Not quite, you'll have to follow a few steps: https://github.com/python-versioneer/python-versioneer#quick-install Regarding the version I don't have a strong preference, but I find 1.0b1 more appropriate.

I had another look #212 and brian-team/brian2genn#101. I still think that the problems that came up there might not be relevant for Brian2CUDA (at least in my hands, I mostly had similar issues with Brian2GeNN, while Brian2CUDA mostly just worked), I would like to try to get a good conda package working. It might even be worth trying out cudatoolkit-dev to ship the entire toolchain.

The issues I had were mostly with cudatoolkit-dev – the idea was that installing the brian2genn package would automatically pull in CUDA, so a user wouldn't have to install anything manually. This never quite worked out, and I got the impression that the package was really only meant to be used as a build-time dependency – there are not that many packages requiring nvcc during runtime I guess 😄 Apart from these issues I am not that sure anymore that depending on the package is really a good idea: do we really want to pull it in unconditionally even for users that already have system-wide CUDA installation? We could have a special additional brian2cuda-with-cuda package that adds the dependency, but this adds quite a bit of maintenance overhead... Depending on nvcc_linux64 is also a bit tricky, since it will automatically install the latest version instead of the version appropriate for the user's machine. So in the end we'll have to ask the user to do something manual in either case. Not quite sure what the advantage of a conda package will be in the end (given that Brian2CUDA is a python-only package), in particular since we cannot really test the package on the conda-forge infrastructure without a GPU...

@mstimberg
Copy link
Member

I added a minimal GitHub Action to build the packages and push them to (Test)PyPI. It seems to work in general, but the pushing step needs you (as the owner of the package on PyPI) to generate API tokens for TestPyPI and PyPI and then add them as "secrets" named test_pypi_password and pypi_password to the GitHub settings. It would probably also make sense to add me (marcelstimberg) as an owner to (Test)PyPI.

@denisalevi
Copy link
Member Author

@mstimberg Thanks, I added you as an owner to TestPyPi (there is no package on PyPi yet, I'll add you as soon as we have a first release that we upload). I'll add the secrets next week when I work on this again.

  1. On TestPyPi, the Brian2 package is called Brian2 instead of brian2. Any specific reason for that, does it make sense to do the same for Brian2CUDA?

We call the package Brian2 in setup.py, and that's why it is called this way on PyPI and TestPyPI. There's no particular reason for this, but PEP 426 guarantees that both names are considered equivalent.

Ah, i didn't know it doesn't matter for the package name. In that case, I would call it Brian2CUDA I suppose.

  1. Brian2 doesn't use GitHub's release option, instead it only uses tags. I saw that the GitHub action for PyPi publishing relies on pushing tags to the repo, is that the reason? Or anything else?

Again, no particular reason, we always used tags. The advantage is that you simply add a tag locally and push it to trigger a release instead of using github's GUI for it. I guess one could now use GitHub's CLI as easily, but well, it's not a high priority to change it, I think. The only advantage I'm seeing is that releases are more visible on GitHub and can have release notes.

I see. I do like the GitHub release visibility tbh. Maybe I'll add an Action to make an official release when pushing a tag. Well, but time... will see ^^

  1. I had a look at the conda receipt and PyPi publish action. There were a few things I didn't understand, like Windows- and Cython-specific setups. If you have time, could you maybe strip both receipts down to the bare minimum (deleting everything Windows- and Cython-specific) and pushing it to this branch? I think that would save me a lot of time and should be quick for you I think?

The conda recipe is out of date and we don't use it anymore (we should probably delete it...). The relevant one for conda-forge is the one here: https://github.com/conda-forge/brian2-feedstock/blob/main/recipe/meta.yaml But it's not guaranteed to correctly works like this on a local machine, it relies on the conda-forge infrastructure. I'll have a look at the PyPI action.

Ah, ok. Thanks. Does it make sense to have that for Brian2CUDA as well? I suppose it is triggered from the PyPi release?

  1. I'd also like to use versioneer, as far as I understand, I just have to copy the Python file over in this repo. Will have a look at that. I would say our first release here will be a beta release. Have to decide if we want to version that as 0.1 and upgrade to 1.0 once we think beta is over? Or go with 1.0b1 (maybe depends also on versioneer?).

Not quite, you'll have to follow a few steps: https://github.com/python-versioneer/python-versioneer#quick-install Regarding the version I don't have a strong preference, but I find 1.0b1 more appropriate.

Okay, will install it next week then. And I'm fine with 1.0b1.

I had another look #212 and brian-team/brian2genn#101. I still think that the problems that came up there might not be relevant for Brian2CUDA (at least in my hands, I mostly had similar issues with Brian2GeNN, while Brian2CUDA mostly just worked), I would like to try to get a good conda package working. It might even be worth trying out cudatoolkit-dev to ship the entire toolchain.

The issues I had were mostly with cudatoolkit-dev – the idea was that installing the brian2genn package would automatically pull in CUDA, so a user wouldn't have to install anything manually. This never quite worked out, and I got the impression that the package was really only meant to be used as a build-time dependency – there are not that many packages requiring nvcc during runtime I guess smile Apart from these issues I am not that sure anymore that depending on the package is really a good idea: do we really want to pull it in unconditionally even for users that already have system-wide CUDA installation? We could have a special additional brian2cuda-with-cuda package that adds the dependency, but this adds quite a bit of maintenance overhead... Depending on nvcc_linux64 is also a bit tricky, since it will automatically install the latest version instead of the version appropriate for the user's machine. So in the end we'll have to ask the user to do something manual in either case. Not quite sure what the advantage of a conda package will be in the end (given that Brian2CUDA is a python-only package), in particular since we cannot really test the package on the conda-forge infrastructure without a GPU...

Okay. For now I think we should just have a basic conda package that depends on Brian2 only (to make it easy to install the correct version of Brian2 + Brian2CUDA in a conda environment; otherwise one installs maybe Brian2 via conda, then Brian2CUDA via pip, which installs a different Brian2 version via pip as dependency and there the mess starts). And forget about the cudatoolkit-dev for now. And mention nvcc_linux64 is the installation docs for users that want to set a specific CUDA version for the conda environment.

@mstimberg
Copy link
Member

Hi @denisalevi. Switching to versions starting with v, the license change, and the citation should be taken care of now.

@mstimberg
Copy link
Member

MANIFEST.in should be good to go as well.

I think the only remaining packaging-related things that need to be done before the release are adding the TestPyPI/PyPI credentials as secrets on GitHub, and adding me as an additional maintainer. Conda package recipe will be trivial once the package is on PyPI (can be created via https://github.com/conda-incubator/grayskull). Maybe best to wait with the mastermain transition until all PRs are merged?

@denisalevi
Copy link
Member Author

Great! Thanks for the help. I'll try to get the last fixes + docs done over the weekend, or if necessary next week.

@denisalevi
Copy link
Member Author

Okay. Credentials are added. Last non-optional TODOs have to be done once this PR is merged (and we uploaded first releases to PyPi).

@mstimberg
Copy link
Member

Looks all good to me 👍
Should we merge it now to see whether the TestPYPi upload works and whether the uploaded package is installable?

@denisalevi
Copy link
Member Author

Sure, the version name might just be weird (last tag being paper2022, but I guess its just on testpypi anyways). I'm merging

@denisalevi denisalevi merged commit dc35a70 into master May 16, 2022
@denisalevi denisalevi deleted the prepare-release branch May 16, 2022 10:08
@mstimberg
Copy link
Member

Sure, the version name might just be weird (last tag being paper2022, but I guess its just on testpypi anyways). I'm merging

It will not consider the paper2022 tag anymore, since it does not start with v. The version will be something like 0.post0.dev1063

@mstimberg
Copy link
Member

https://test.pypi.org/project/Brian2CUDA will continue to redirect to 0.1 because it is considered newer than 0.post0...., but that shouldn't be a problem. It is only for testing anyway. The upload worked fine: https://test.pypi.org/project/Brian2CUDA/0.post0.dev1063/

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

Successfully merging this pull request may close these issues.

None yet

2 participants