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

Version should follow CalVer #183

Closed
giovannipizzi opened this issue Apr 22, 2021 · 8 comments · Fixed by #185
Closed

Version should follow CalVer #183

giovannipizzi opened this issue Apr 22, 2021 · 8 comments · Fixed by #185
Assignees

Comments

@giovannipizzi
Copy link
Contributor

(As discussed in person with @chrisjsewell)

So the version should be (yy.mm.xx, e.g. 21.04.xx), and not dd.mm.yy; where yy is the 2-digit year, and mm the 2-digit month.
Moreover, typically xx is just a sequential number within the month, so the first april 2021 version is 21.04.0, then 21.04.1 etc.

This is important for sorting correctly the versions, among other things (but also, very simply, for consistency with the version names of earlier versions).

Also, please check what was done earlier, and if there was a v or not in front of the version (and where - there might be differences between the git tags (that have it) and the actual name of the version of the OVA (that does not), again for consistency and proper sorting of versions. Check earlier versions e.g. here and here, and tags here.

As an additional note, since the discussion for CalVer vs. SemVer popped out: I think for QMobile (as well as for other bit software collections) it makes more sense to (continue to) use CalVer (this was a conscious choice, pinging also @ltalirz) as you want to know "this is the version of April 2021", that implicitly tells you which version of all the software included you might expect to find, and (if you already have it installed) how "old" the VM is. Also, for Quantum Mobile, there is much less the concept of "backward-incompatible" changes, like in SemVer. Finally, a SemVer for Quantum Mobile risks to be confusing with the versions of the codes inside.

For a code, instead, using SemVer makes more sense (e.g. AiiDA, a Quantum code, ...).
Therefore, I would strongly advise to continue using CalVer for QMobile, or have a discussion if there are good reasons to change that are not listed here.

@ltalirz
Copy link
Member

ltalirz commented Apr 22, 2021

fully agree. good thing is you can still edit the name of the release on github and you can simply tag the same commit with the correct string (even without deleting the old one)

@chrisjsewell
Copy link
Member

chrisjsewell commented Apr 22, 2021

Yes my bad that was just a mistake for 16.04.21a

If we are using calver, which is fine, then lets use it properly with full years so that it is clear to people that this is a date: 2021.04.xx

This is important for sorting correctly the versions

The releases already get sorted by date not version identifier, so TBH I don't really think this is so much of an issue. None of the OVAs have a v prefix for the version, I added v to the git tags, to discriminate them as release tags but can remove them.

Note the known "issues" for this release are

  1. abinit is not compiled with a parallel version of netcdf (would require a full compilation of netcdf, as opposed to the current apt install)
  2. cp2k is a serial distribution (which has always been the case)
  3. AiiDALab is present and fully working but, as per Nicola's request, the desktop launch button was removed.

For (1) and (2) I really don't want to waste anymore time on this, if we are anyhow moving to Conda installs (#161) (plus I did not have any further feedback on them from those responsible for these codes)

Additionally, for the common-workflow package, it would potentially be possible to include this in the distribution. But only really if the plugin dependencies of that package were moved to extras, so that they did not clobberthe existing versions of installed packages, or if it was installed in a separate virtual environment.
I also asked Marnik to look into adding a CLI command in that package, which could launch minimal runs of all (or a subset of) plugins (e.g. using the lowest protocol against the simplest structure), as a form of integration testing.

@giovannipizzi
Copy link
Contributor Author

Pinging @sphuber as he's looking into ensuring that all codes work in the aiida-common-workflow package. I think we're fine with 3, and for 1 and 2 we can keep the current binaries unless we find a fix in the next few days, and we'll add a note. But we'll need to the aiida-common-workflows in the next release (~1 week?). The other issue to fix before release is #171

@giovannipizzi
Copy link
Contributor Author

Yes my bad that was a mistake for 16.04.21a

No problem, it can happen. @ltalirz suggested some fixes above

If we are using calver, which is fine, then lets use it properly with full years so that it is clear to people that this is a date: 2021.04.xx

While I agree it makes it clearer, I'm not sure this is "needed", e.g. on https://calver.org it shows both options are used (notable example of 2-digit year: pip).
Let's discuss if this is needed. In principle that's ok to do as 2021 > 21 so sorting remains OK, but this is "non-reversible", as we cannot anymore change our mind and go back to 2-digit.

The releases already get sorted by date not version identifier, so TBH I don't really think this is so much of an issue.

Yes and no - it does not matter on the web interface, but it matters if a scripts tries to figure out the most recent released version. We don't have a pip package luckily :-) but people might try to get e.g. the most recent docker image if we have or will have one at some point e.g. on docker hub.
See e.g. this gist and the output of this code:

13.1.1
16.04.21
20.01.2
21.01.0
21.01.1

None of the OVAs have a v prefix for the version, I added v to the git tags, to discriminate them as release tags but can remove them.

No, I think that's OK and should be consistent with earlier versions (v for tags and not for versions) - I just meant to double check we're consistent. The only thing that has changed is in the releases page, where the most recent versions are:

  • Quantum Mobile (16.04.21a)
  • v21.03.18-qe
  • v20.11.2a
  • ...

So the most recent follows a different naming scheme.

@chrisjsewell
Copy link
Member

But we'll need to the aiida-common-workflows in the next release (~1 week?).

then the key things ideally is that (a) it needs to be able to be installed without intefering with the existing environment (this is exactly what I've just been through with aiidalab, where I basically got them to rewrite it to not enforce non-essential dependencies and remove absolute pinnings 😬), like aiidalab, aiida-common-workflows will be installed with a constraints file, enforcing it can not change the aiida-core version, its dependencies or the plugin versions (b) it should implement a "testing" CLI command, to run a "minimal" computation for each plugin, that QM can use an integration test that the plugins and their codes are nominally working

@sphuber
Copy link

sphuber commented Apr 23, 2021

then the key things ideally is that (a) it needs to be able to be installed without intefering with the existing environment (this is exactly what I've just been through with aiidalab, where I basically got them to rewrite it to not enforce non-essential dependencies and remove absolute pinnings grimacing), like aiidalab, aiida-common-workflows will be installed with a constraints file, enforcing it can not change the aiida-core version, its dependencies or the plugin versions

This is going to be tricky. Best I can do is to have just a lower requirement. Since we are actively developing these plugins, for them to work with the aiida-common-workflows package, we often need the latest version. Is this problematic @chrisjsewell ? Why would the QM care what version is installed if it is the most recent?

(b) it should implement a "testing" CLI command, to run a "minimal" computation for each plugin, that QM can use an integration test that the plugins and their codes are nominally working

This is more or less there, but not for all plugins and we cannot provide this for all plugins, because not all codes are freely distributed and can therefore be included in the QM. Even for some plugins where it is possible (i.e. Siesta, Abinit and QE) but it needs some additional resources to be installed, i.e. pseudo potentials. At least for the latter two, this is straightforward as it can be done in a single command through the aiida-pseudo CLI which is installed as a dependency. Of course this is all about integration tests. The best would maybe to just run the unit tests, or is that not what you are after?

@chrisjsewell
Copy link
Member

Why would the QM care what version is installed if it is the most recent?

when it breaks the environment, as was previously happening with aiida-fleur. QM specifically sets (and documents) the plugin versions, aiida-common-workflows should not override that

because not all codes are freely distributed and can therefore be included in the QM

Yeh the CLI command would allow for specifying which plugin(s) to run for

Even for some plugins where it is possible (i.e. Siesta, Abinit and QE) but it needs some additional resources to be installed, i.e. pseudo potentials

well I would think we may want these resources to be pre-installed on QM

The best would maybe to just run the unit tests, or is that not what you are after?

Not really; the unit tests obviously do not run against the actual simulation codes. I want something that will properly run a (minimal) end-to-end computation with the aiida codes

@sphuber
Copy link

sphuber commented Apr 26, 2021

when it breaks the environment, as was previously happening with aiida-fleur. QM specifically sets (and documents) the plugin versions, aiida-common-workflows should not override that

I can see that, but that will be practically impossible at this stage. If we cannot have the latest version of most if not all plugins, the common workflows are simply broken and so pointless to install them. Who else has very specific requirements on plugins?

Yeh the CLI command would allow for specifying which plugin(s) to run for

The CLI commands do already allow to specify a specific plugin so that is ok. Currently there are only commands to submit a full workchain, so the test would have to require a running daemon etc.

well I would think we may want these resources to be pre-installed on QM

I am all for that. Both Abinit and QE use aiida-pseudo so installing them will be just a simple CLI command. For Siesta it still goes through the old verdi data upf uploadfamily but I think the pseudos are already available on QM, so it should also be straightforward.

Not really; the unit tests obviously do not run against the actual simulation codes. I want something that will properly run a (minimal) end-to-end computation with the aiida codes

Ok then the CLI is exactly what you are looking for

@chrisjsewell chrisjsewell linked a pull request Apr 29, 2021 that will close this issue
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 a pull request may close this issue.

4 participants