search

aiidateam / aiida-common-workflows

A repository for the implementation of common workflow interfaces across materials-science codes and plugins
https://aiida-common-workflows.readthedocs.io
MIT License
52 stars 32 forks source link

Wien2k implementation #314

Closed sphuber closed 1 year ago

sphuber commented 1 year ago

The tests fail because the required wien2k plugin is not installed. @rubel75 @bosonie where is the aiida-wien2k plugin located?

sphuber commented 1 year ago

supercedes #313

bosonie commented 1 year ago

The aiida-wien2k is now owned by aiida-team, @mbercx and I are in charge to take care of the release. I will coordinate with him. Regarding this PR, for me it is the same to work here or merge this and open a new one. As you wish.

sphuber commented 1 year ago

Let's just add a commit on to this branch once the plugin is done and released. This way we can be sure the tests pass and don't break the master branch.

mbercx commented 1 year ago

Tests for 3.10 (see this action run) seem to be failing because of

https://github.com/yaml/pyyaml/issues/724

I tried installing pyyaml==5.4.1 in a 3.10 environment on Ubuntu and am running into the same issue. (Funny enough, it works fine on mac). So we may have to update our dependencies for aiida-core 1.6.X.

After removing 3.10 from the test matrix though, the tests immediately fail because of abipy, see:

https://github.com/abinit/abipy/issues/263

mbercx commented 1 year ago

Alright, at least the Python 3.9 tests pass now! For Python 3.10, I think we need to update the pyyaml dependency of aiida-core v1.6.X. Python 3.8 is not supported by the latest version of abipy, so we may need to restrict to Python 3.9 and above.

sphuber commented 1 year ago

Alright, at least the Python 3.9 tests pass now! For Python 3.10, I think we need to update the pyyaml dependency of aiida-core v1.6.X. Python 3.8 is not supported by the latest version of abipy, so we may need to restrict to Python 3.9 and above.

That would essentially mean that we only support 3.9 because aiida-core==1.6.9 officially only supports 3.9: https://github.com/aiidateam/aiida-core/blob/v1.6.9/setup.json

I think this is quite limited and not ideal for a major release of this package. It would be great if we can check whether aiida-core v1.6.9 can add support for Python 3.10 and maybe 3.11 and also check whether abipy can add support for Python 3.7 or at least Python 3.8. Are they actually using functionality that is only introduced in Python 3.9? If so what, there are a number of modules that have backports.

mbercx commented 1 year ago

It would be great if we can check whether aiida-core v1.6.9 can add support for Python 3.10 and maybe 3.11

Yes, that's also was I was thinking. I hope it will only require use to update the pyyaml dependency. Do you have time to look into this?

abipy can add support for Python 3.7 or at least Python 3.8.

I'll look into that. Python 3.7 is already past EOL, so I think we can skip that (aiida-abinit also doesn't support it), but Python 3.8 may be feasible for abipy. I hope it will only require some from __future__ import annotations additions.

mbercx commented 1 year ago

Seems I only had to add one from __future__ import annotations to make our tests pass. Will open a PR into abipy to reintroduce support for Python 3.8.

sphuber commented 1 year ago

Yes, that's also was I was thinking. I hope it will only require use to update the pyyaml dependency. Do you have time to look into this?

If only it were so simple. The same dependency has also been pinned by plumpy which in turn relies on kiwipy. So these may also require new support releases. But I can try to have a look.

sphuber commented 1 year ago

@mbercx I solved the kiwipy and plumpy issues in this branch but as I expected, the real problems only start then. The installation runs for Python 3.7, 3.8 and 3.9, but the transport tests fail. This can most likely be fixed, however, for Python 3.10 and 3.11 the installation doesn't even succeed. This is again due to compilation errors when building pymatgen. These are very tricky to solve if even possible and I don't have the time nor interest currently to work on this, just to bring Python 3.10 and 3.11 support to an old version of AiiDA that has been out of support for a while. Think we will have to live with support for just Python 3.8 and 3.9 for this release of ACWF. We can then update to AiiDA v2.0 a.s.a.p and support more modern Python versions

mbercx commented 1 year ago

@sphuber of course, I fully understand. Thanks for having a look anyways!

bosonie commented 1 year ago

@mbercx I fixed the small conflict about the abipy version in pyproject.toml. Do you agree then that we merge this? We can keep testing and if some other issue comes up we can do another PR. At least we have a working implementation.

mbercx commented 1 year ago

@bosonie sure, we can go ahead and (Squash merge) this PR. I can confirm it runs WIEN2k just fine, just having trouble getting a working binary.