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

Release v2.4.1 #6176

Merged
merged 13 commits into from
Nov 15, 2023
Merged

Release v2.4.1 #6176

merged 13 commits into from
Nov 15, 2023

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Nov 14, 2023

No description provided.

The current Docker image provided with `aiida-core` depends on
`aiida-prerequisites` as a base image. This image is maintained outside
of the `aiida-core` repo, making it additional maintenance to keep it up
to date when a new `aiida-core` version is released. In addition, the
`aiida-prerequisites` image is no longer maintained because the AiiDAlab
stack now depends on another base image.

Finally, the `aiida-prerequisites` design had shortcomings as to how the
required services, PostgreSQL and RabbitMQ, are handled. They had to be
started manually and were not cleanly stopped on container shutdown.

An AEP was submitted to add two Docker images to `aiida-core` that
simplifies their maintenance and that improve the usability by properly
and automatically handling the services. See the AEP for more details:
https://aep.readthedocs.io/en/latest/009_improved_docker_images/readme.html

Cherry-pick: 9e80834
The buildjet arm64 runner has only three-month trials, after that
we need to pay to use it. The self-hosted runner is deployed on
the macOS-arm64 machine located in PSI.

Cherry-pick: 34be3b6
The docker build workflow was only activated when changes were made to
either the `.docker` directory or `.github/workflows/docker*.yaml` files.
However, changes in the `aiida` package could also break the build and
so could pass by unnoticed.

The trigger conditions are changed to instead trigger always except for
changes to the `tests` and `docs` directories.

Cherry-pick: 1b5b669
Amendment to #6139, for unknown reason, docker pull is failed for
docker.io on this repository. Using the docker registry ghcr.io works
fine.

Cherry-pick: 8d8b41d
It was still pointing to the old name instead of the new `aiida-core-with-services`.

Cherry-pick: 5ca609b
Set `with-contenv` such that environment variables are forwarded. Without
this, settings like the work dir of `localhost` will be set incorrectly and will cause
calculations to fail.

Cherry-pick: 6123f52
…n start (#6170)

In order to simplify the implementation of using the `aiida-core` image
as the base for customized images, the `run-before-daemon-start` and
`run-after-daemon-start` script folders are created. Any executables in
these two folders will be executed before and after the AiiDA daemon is
started in the container, respectively.

The standard linux `run-parts` tool is used to scan these folders for
files, which are run in the lexical sort order of their names, according
to the C/POSIX locale character collation rules

Cherry-pick: 5fe6f4f
Copy link

codecov bot commented Nov 14, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (037f238) 79.53% compared to head (770ef25) 79.52%.

Files Patch % Lines
aiida/orm/nodes/data/structure.py 69.24% 4 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           support/2.4.x    #6176      +/-   ##
=================================================
- Coverage          79.53%   79.52%   -0.00%     
=================================================
  Files                546      546              
  Lines              39687    39697      +10     
=================================================
+ Hits               31560    31565       +5     
- Misses              8127     8132       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…ns (#6088)

The roundtrip test for the `StructureData` class using `pymatgen`
structures as a go between started failing. The structure is constructed
from a CIF file with partial occupancies. The `label` attribute of each
site in the pymatgen structure, as returned by `as_dict` would look like
the following, originally:

    ['Bi', 'Bi', 'Te:0.667, Se:0.333', 'Te:0.667, Se:0.333', 'Te:0.667, Se:0.333']
    ['Bi', 'Bi', 'Te:0.667, Se:0.333', 'Te:0.667, Se:0.333', 'Te:0.667, Se:0.333']

In commit 63bbd23b57ca2c68eaca07e4915a70ef66e13405, released with
v2023.7.14, the CIF parsing logic in `pymatgen` was updated to include
parsing of the atom site labels and store them on the site `label`
attribute. This would result in the following site labels for the
structure parsed directly from the CIF and the one after roundtrip
through `StructureData`:

    ['Bi', 'Bi', 'Se1', 'Se1', 'Se1']
    [None, None, None, None, None]

The roundtrip returned `None` values because in the previously mentioned
commit, the newly added `label` property would return `None` instead of
the species label that used to be returned before. This behavior was
corrected in commit 9a98f4ce722299d545f2af01a9eaf1c37ff7bd53 and released
with v2023.7.20, after which the new behavior is the following:

    ['Bi', 'Bi', 'Se1', 'Se1', 'Se1']
    ['Bi', 'Bi', 'Te:0.667, Se:0.333', 'Te:0.667, Se:0.333', 'Te:0.667, Se:0.333']

The site labels parsed from the CIF are not maintained in the roundtrip
because the `StructureData` does not store them. Therefore when the final
pymatgen structure is created from it, the `label` is `None` and so
defaults to the species name.

Since the label information is not persisted in the `StructureData` it
is not guaranteed to be maintained in the roundtrip and so it is
excluded from the test.

Cherry-pick: d1d64e8
As of v2023.9.2, the ``properties`` argument of the `Specie` class is
removed and the ``spin`` argument should be used instead. See:
materialsproject/pymatgen@118c245

The ``spin`` argument was introduced in v2023.6.28. See:
materialsproject/pymatgen@9f2b393

Instead of removing support for versions older than v2023.6.28 the code
is updated to be able to deal with the new version where `properties` is
no longer supported.

Cherry-pick: 4e0e7d8
The `test_unload_profile` test verifies that if a loaded profile is
unloaded, it properly relinquishes of the session that is maintained by
sqlalchemy. It did so by checking that after unloading, there were no
sessions being referenced. However, this would fail sometimes, because
another session may still be held on to, even though that session had
nothing to do with the test.

A more robust test is simply to check that after unloading, there is
exactly one less session being held on to.

Cherry-pick: 1c72eac
@unkcpz
Copy link
Member

unkcpz commented Nov 14, 2023

I test the image locally on my mac M1 and works fine. The image used in aiidateam/aiida-quantumespresso#981 also works as expected.

@sphuber sphuber merged commit f392459 into support/2.4.x Nov 15, 2023
48 of 49 checks passed
@sphuber sphuber deleted the release/2.4.1 branch November 15, 2023 07:50
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.

2 participants