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

⬆️ Update to aiida-core v1.4.2 #48

Merged
merged 17 commits into from
Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .ansible-lint
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
warn_list: # or 'skip_list' to silence them completely
# use `# noqa xxx` at the end of a line, to ignore a particular error
# or add to the warn_list, to ignore for the whole project
warn_list:
- '106' # Role name {} does not match ``^[a-z][a-z0-9_]+$`` pattern
- '208' # File permissions unset or incorrect
- '305' # Use shell only when shell functionality is required
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
*.swp
.DS_Store
.galaxy_install_info
.vscode/
.tox/
13 changes: 8 additions & 5 deletions defaults/main.yml
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
aiida_version: "1.3.0"
aiida_version: "1.4.2"
aiida_tag: v{{ aiida_version }}
aiida_extras:
- rest
- docs
- atomic_tools
- testing
- notebook
aiida_data_folder: "${HOME}/.local/share/aiida"
aiida_templates_folder: "${HOME}/.local/share/aiida"
aiida_source_folder: "${HOME}/src"
aiida_examples_folder: "${HOME}"
aiida_venv: "${HOME}/.virtualenvs/aiida"
aiida_constraints_file: "{{ aiida_data_folder }}/constraints.txt"
# these are additional to those extracted from aiida-core and its extras
aiida_constraints:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be removed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see; we need to lock things down even further (?).
Should we be adding this constraint in the setup.json?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps 🤷 (to the docs extra), I guess with the new pip --use-feature=2020-resolver it should resolve this and not allow such a broken environment to transpire. But without it, and with this particular set of extras it definitely creates a broken environment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisjsewell that's why I advocate for having proper env requirements+pins managed by pip-tools instead of extras

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh we do already have lock files for CI testing: https://github.com/aiidateam/aiida-core/tree/develop/requirements.
The problem is that we are not just trying to install aiida-core, we need to install a range of other plugin packages that might not be exactly compatible with these hard pinnings:

aiida_plugin_versions:
aiida_cp2k: "1.1.0"
aiida_fleur: "1.1.0"
aiida_bigdft: "0.2.1a2"
# aiida_gudhi: "0.1.0a3"
# aiida_phtools: "0.1.0a1"
aiida_quantumespresso: "3.1.0"
# aiida_raspa: "1.0.0a2"
aiida_siesta: "1.1.0"
aiida_yambo: "1.1.1"
aiida_wannier90: "2.0.1"
aiida_wannier90_workflows: "1.0.1"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, so you need a combined lockfile, I guess... Just dump that to a file and use a few requirements sources to produce transitive pins? FWIW I think what confused me is the definition substitution (constraints vs requirements).

Copy link
Member Author

@chrisjsewell chrisjsewell Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh exactly, I'm not completely sold on the current solution here, but at least now it is properly tested and produces a valid environment, which is a step forward 😬
So I am going to merge this as this, but anyway we have an open issue marvel-nccr/quantum-mobile#137 to consider this further, at which point I will definitely take into account your recommendation (thanks 🙏) and may well switch to some form of lock file

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @webknjaz you might be interested to see I now set up a full ~2 hour vagrant+ansible build running in GH Actions: https://github.com/marvel-nccr/quantum-mobile/runs/1286287172?check_suite_focus=true 😄

- "nbconvert<6.0" # required by notebook 5.7.10

# profile
aiida_profile_name: generic
Expand Down Expand Up @@ -58,15 +61,15 @@ aiida_components:
# Note: If possible, these versions should coincide with the ones in
# https://github.com/aiidalab/aiidalab-metapkg/blob/master/requirements.txt
aiida_plugin_versions:
aiida_cp2k: "1.1.0"
aiida_cp2k: "1.2.0"
aiida_fleur: "1.1.0"
aiida_bigdft: "0.2.1a2"
# aiida_gudhi: "0.1.0a3"
# aiida_phtools: "0.1.0a1"
aiida_quantumespresso: "3.1.0"
aiida_quantumespresso: "3.2.0"
# aiida_raspa: "1.0.0a2"
aiida_siesta: "1.1.0"
aiida_yambo: "1.1.1"
aiida_yambo: "1.1.3"
aiida_wannier90: "2.0.1"
aiida_wannier90_workflows: "1.0.1"

Expand Down
37 changes: 37 additions & 0 deletions files/aiida-core-constraints.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#!/usr/bin/env python
Copy link
Member

@ltalirz ltalirz Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine to keep it this parser here for the moment - eventually this should probably go to into https://github.com/aiidateam/aiida-core/tree/develop/utils

"""Script to convert aiida setup.json to pip constraints file.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not constraints but rather requirements. You'd usually use https://github.com/jazzband/pip-tools to generate the pinned env deps aka constraints.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, pip-tools can generate pins from setup.py natively so you could just replace this script with an existing tool: https://github.com/jazzband/pip-tools#requirements-from-setuppy

Copy link
Member Author

@chrisjsewell chrisjsewell Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no that creates a lock file, with strict version pinning. we just want a constraints file, for when you run subsequent pip installs, i.e. these installs can change the version of packages if necessary, but only within the ranges specified by setup.json

Copy link

@webknjaz webknjaz Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisjsewell true but the purpose of constraints file in pip is exactly that it should be a lock file. So that the usage would be pip install -r requirements.txt -c constraints.txt. The former should usually contain open-ended direct dependencies while the latter would pinpoint what exactly is allowed (it's best when there's also --require-hashes too), including the transitive deps too, making the virtualenv setup reproducible.


Usage::

python aiida-core-constraints.py path/to/setup.json extras1 extras2 ...
"""
import json
from pathlib import Path
import sys

def remove_extra(req):
"""Extras are not allowed in constraints files, e.g. dep[extra]>0.1 -> dep>0.1 """

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, fair enough. Using pip-tools would fix that too.

if "[" not in req or "]" not in req:
return req
start, end = req.split("[", 1)
return start + end.split("]", 1)[1]


def main(path, *extras):
path = Path(path)
data = json.loads(path.read_text("utf8"))

output = ["# aiida-core requirements: v" + data["version"]]
# we can't add this when using the file as a constraint file for actually installing aiida-core
# output.append("aiida-core==" + data["version"])
output.extend([remove_extra(req) for req in data["install_requires"]])

for extra in extras:
output.append("# " + extra)
output.extend([remove_extra(req) for req in data["extras_require"][extra]])

print("\n".join(output))

if __name__ == "__main__":
assert len(sys.argv) > 1, "JSON path required"
main(sys.argv[1], *sys.argv[2:])
6 changes: 6 additions & 0 deletions molecule/default/converge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

vars:
ansible_python_interpreter: /usr/bin/python3
aiida_venv: "${HOME}/.virtualenvs/aiida"

tasks:
- name: Create dummy pw executable
Expand All @@ -28,3 +29,8 @@
folder: "/usr/bin"
executable: "pw.x"
plugin: quantumespresso.pw

post_tasks:
- name: run pip check
command: "{{ aiida_venv }}/bin/pip check --no-color"
changed_when: false
10 changes: 10 additions & 0 deletions molecule/default/verify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@
- item.name in aiida_pps.stdout
with_items: "{{ aiida_pseudopotentials }}"

- name: print aiida-core version
shell: "{{ aiida_venv }}/bin/pip show aiida-core"
changed_when: false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you don't even need this in verify.yml (of course fine to keep)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought just in case a plugin tries to install a different version

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I meant the changed_when is not needed (it's fine to change things in verify.yml)

register: aiida_pip_show

- name: check aiida-core version
assert:
that:
- aiida_version in aiida_pip_show.stdout

- name: check plugin versions
include_tasks: tests/test_plugins.yml
with_dict: "{{ aiida_plugin_versions }}"
6 changes: 3 additions & 3 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# for running
ansible~=2.9
ansible~=2.10.0
# for testing
docker
molecule~=3.0
molecule[docker]~=3.1.0
docker~=4.2
19 changes: 19 additions & 0 deletions tasks/aiida-core.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,24 @@
version: "{{ aiida_tag }}"
register: git_checkout

- name: Copy pip constrain generation script to folder
copy:
src: aiida-core-constraints.py
dest: "{{ aiida_source_folder }}/aiida-core"

- name: Create pip constraints file contents
command: >
/usr/bin/python3 '{{ aiida_source_folder }}/aiida-core/aiida-core-constraints.py'
{{ aiida_source_folder }}/aiida-core/setup.json
{{ aiida_extras | join(' ') }}
changed_when: false
register: constraints

- name: Write pip constraints file
copy:
dest: "{{ aiida_constraints_file }}"
content: "{{ constraints.stdout }}\n# additional\n{{ aiida_constraints | join('\n') }}"

- name: Install AiiDA git code
pip:
name:
Expand All @@ -14,6 +32,7 @@
# According to https://github.com/ansible/ansible/issues/52275
virtualenv_command: /usr/bin/python3 -m venv
editable: true
extra_args: "-c {{ aiida_constraints_file }}"
register: aiida_core_install
notify: reentry scan
# this guard is necessary because, for some reason, installs with
Expand Down
4 changes: 2 additions & 2 deletions tasks/aiida-prepare.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@
pip:
name:
- pip~=20.0
- setuptools~=36.2
- setuptools
- wheel
- numpy==1.17.4
- numpy # required by pymatgen
Copy link
Member

@ltalirz ltalirz Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no pinning, are you sure? ;-)

I'd feel safer with pinning it to some recent minor version

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is only needed for the pymatgen install, it will be updated, if necessary, by the pip install of aiida-core. If you pin it, then you have to ensure it is compatible with aiidalab and aiida-core (a current issue in the full quantum mobile build)

virtualenv: "{{ aiida_venv }}"
# According to: https://github.com/ansible/ansible/issues/52275
virtualenv_command: /usr/bin/python3 -m venv
Expand Down
20 changes: 10 additions & 10 deletions tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,47 +13,47 @@
- name: Get info on current user
include_role:
name: marvel-nccr.current_user
tags: always
tags: [always]
when: current_user is undefined

# apt and python packages
- include_tasks: aiida-deps-rhel.yml
tags: aiida_deps
tags: [aiida_deps]
when: ansible_os_family|lower == 'redhat'
- include_tasks: aiida-deps-debian.yml
tags: aiida_deps
tags: [aiida_deps]
when: ansible_os_family|lower == 'debian'

# Start and setup Postgresql and RabbitMQ, create folders
- import_tasks: aiida-prepare.yml
tags: aiida, aiida_prepare
tags: [aiida, aiida_prepare]

# install and setup AiiDA, including profile etc.
- import_tasks: aiida-core.yml
tags: aiida,aiida_core
tags: [aiida, aiida_core]
- import_tasks: aiida-daemon.yml
tags: aiida_daemon,aiida_core
- import_tasks: aiida-rest.yml
tags: aiida_rest,aiida_core
tags: [aiida_rest, aiida_core]

# setup computers
- include_tasks: computers-setup.yml
tags: aiida_computers,aiida_core
tags: [aiida_computers, aiida_core]
when: '"computers" in aiida_components'

# setup plugins and codes
- include_tasks: plugins/main.yml
tags: aiida_plugins
tags: [aiida_plugins]
when: '"plugins" in aiida_components'

# Set up pseudopotentials
- include_tasks: aiida-pps.yml
vars:
pp: "{{ item }}"
with_items: "{{ aiida_pseudopotentials }}"
tags: aiida_pps
tags: [aiida_pps]
when: '"pseudopotentials" in aiida_components'

- include_tasks: aiida-examples.yml
tags: aiida_examples
tags: [aiida_examples]
when: '"examples" in aiida_components'
6 changes: 3 additions & 3 deletions tasks/plugins/aiida-bigdft.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
name: aiida-bigdft
version: "{{ aiida_bigdft_version }}"
virtualenv: "{{ aiida_venv }}"
extra_args: "-c {{ aiida_constraints_file }}"
register: pip_install

- name: reentry scan
Expand All @@ -27,8 +28,7 @@
- include_role:
name: release_notes
vars:
section: AiiDA
section: "AiiDA Plugins"
option: aiida-bigdft
value: >-
aiida-bigdft {{ aiida_bigdft_version }} is installed.
value: "{{ aiida_bigdft_version }}"
when: release_notes is defined and release_notes
6 changes: 3 additions & 3 deletions tasks/plugins/aiida-cp2k.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
name: aiida-cp2k
version: "{{ aiida_cp2k_version }}"
virtualenv: "{{ aiida_venv }}"
extra_args: "-c {{ aiida_constraints_file }}"
register: pip_install

- name: reentry scan
Expand All @@ -27,8 +28,7 @@
- include_role:
name: release_notes
vars:
section: AiiDA
section: "AiiDA Plugins"
option: aiida-cp2k
value: >-
aiida-cp2k {{ aiida_cp2k_version }} is installed.
value: "{{ aiida_cp2k_version }}"
when: release_notes is defined and release_notes
6 changes: 3 additions & 3 deletions tasks/plugins/aiida-fleur.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
name: aiida-fleur
version: "{{ aiida_fleur_version }}"
virtualenv: "{{ aiida_venv }}"
extra_args: "-c {{ aiida_constraints_file }}"
register: pip_install

- name: reentry scan
Expand All @@ -27,8 +28,7 @@
- include_role:
name: release_notes
vars:
section: AiiDA
section: "AiiDA Plugins"
option: aiida-fleur
value: >-
aiida-fleur {{ aiida_fleur_version }} is installed.
value: "{{ aiida_fleur_version }}"
when: release_notes is defined and release_notes
7 changes: 3 additions & 4 deletions tasks/plugins/aiida-gudhi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pip:
name: git+https://github.com/ltalirz/aiida-gudhi@v{{ aiida_gudhi_version }}
virtualenv: "{{ aiida_venv }}"
extra_args: "-c {{ aiida_constraints_file }}"
register: pip_install

- name: reentry scan
Expand All @@ -26,9 +27,7 @@
- include_role:
name: release_notes
vars:
section: AiiDA
section: "AiiDA Plugins"
option: aiida-gudhi
value: >-
aiida-gudhi v{{ aiida_gudhi_version }} is installed.
See 'verdi code list' for available codes.
value: "{{ aiida_gudhi_version }}"
when: release_notes is defined and release_notes
7 changes: 3 additions & 4 deletions tasks/plugins/aiida-phtools.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pip:
name: git+https://github.com/ltalirz/aiida-phtools@v{{ aiida_phtools_version }}
virtualenv: "{{ aiida_venv }}"
extra_args: "-c {{ aiida_constraints_file }}"
register: pip_install

- name: reentry scan
Expand All @@ -26,9 +27,7 @@
- include_role:
name: release_notes
vars:
section: AiiDA
section: "AiiDA Plugins"
option: aiida-phtools
value: >-
aiida-phtools v{{ aiida_phtools_version }} is installed.
See 'verdi code list' for available codes.
value: "{{ aiida_phtools_version }}"
when: release_notes is defined and release_notes
6 changes: 3 additions & 3 deletions tasks/plugins/aiida-quantumespresso.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
name: aiida-quantumespresso
version: "{{ aiida_quantumespresso_version }}"
virtualenv: "{{ aiida_venv }}"
extra_args: "-c {{ aiida_constraints_file }}"
register: aiida_qe_installed

- name: reentry scan
Expand All @@ -29,8 +30,7 @@
- include_role:
name: release_notes
vars:
section: "AiiDA"
section: "AiiDA Plugins"
option: "aiida-quantumespresso"
value: >-
aiida-quantumespresso {{ aiida_quantumespresso_version }} is installed.
value: "{{ aiida_quantumespresso_version }}"
when: release_notes is defined and release_notes
7 changes: 3 additions & 4 deletions tasks/plugins/aiida-raspa.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pip:
name: git+https://github.com/yakutovicha/aiida-raspa@v{{ aiida_raspa_version }}
virtualenv: "{{ aiida_venv }}"
extra_args: "-c {{ aiida_constraints_file }}"
register: pip_install

- name: reentry scan
Expand All @@ -26,9 +27,7 @@
- include_role:
name: release_notes
vars:
section: AiiDA
section: "AiiDA Plugins"
option: aiida-raspa
value: >-
aiida-raspa v{{ aiida_raspa_version }} is installed.
See 'verdi code list' for available codes.
value: "{{ aiida_raspa_version }}"
when: release_notes is defined and release_notes
6 changes: 3 additions & 3 deletions tasks/plugins/aiida-siesta.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
name: aiida-siesta
version: "{{ aiida_siesta_version }}"
virtualenv: "{{ aiida_venv }}"
extra_args: "-c {{ aiida_constraints_file }}"
register: pip_install

- name: reentry scan
Expand All @@ -27,8 +28,7 @@
- include_role:
name: release_notes
vars:
section: AiiDA
section: "AiiDA Plugins"
option: aiida-siesta
value: >-
aiida-siesta {{ aiida_siesta_version }} is installed.
value: "{{ aiida_siesta_version }}"
when: release_notes is defined and release_notes
Loading