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

👌 IMPROVE: add siesta_build_cpus #14

Merged
merged 8 commits into from
Dec 6, 2020
Merged

Conversation

chrisjsewell
Copy link
Member

No description provided.

defaults/main.yml Outdated Show resolved Hide resolved
@chrisjsewell
Copy link
Member Author

@albgar I've added something similar to some other roles.
do you think this makes sense, and/or will it even effect build times?

@chrisjsewell
Copy link
Member Author

do you think this makes sense, and/or will it even effect build times?

Looks faster to me 😄

before (91b7e51, 1 cpu):

marvel-nccr.siesta : Make siesta executable --------------------------- 191.87s

after (5b6c60d, 2 cpus):

marvel-nccr.siesta : Make siesta executable --------------------------- 102.61s

@chrisjsewell
Copy link
Member Author

Yeh that makes the CI run a lot quicker!
So I'm going to merge now, to get the into QM, but let me know if you see any issues @albgar

@chrisjsewell chrisjsewell merged commit de431b1 into master Dec 6, 2020
@chrisjsewell chrisjsewell deleted the add/siesta_build_cpus branch December 6, 2020 15:59
@albgar
Copy link
Collaborator

albgar commented Dec 6, 2020

@chrisjsewell: The new cpu setting looks fine for the CI, but I wonder what happens if the QM being built has only one cpu.

@chrisjsewell
Copy link
Member Author

chrisjsewell commented Dec 6, 2020

no this is the reason to have it, so you specify directly the number of CPUs the VM has in playbook-build.yml:

  - role: marvel-nccr.libxc  # this is used by abinit and siesta
    tags: [abinit,siesta]
    vars:
      libxc_build_cpus: "{{ vm_cpus }}"

  - role: marvel-nccr.siesta
    tags: [siesta]
    vars:
      siesta_build_cpus: "{{ vm_cpus }}"
      siesta_libxc_root: "{{ libxc_prefix }}"

  - role: marvel-nccr.abinit
    tags: [abinit]
    vars:
      abinit_build_cpus: "{{ vm_cpus }}"
      abinit_libxc_path: "{{ libxc_prefix }}"

@chrisjsewell
Copy link
Member Author

chrisjsewell commented Dec 6, 2020

it will then adapt to how many you set in inventory.yml; 1, 2 or even more

@albgar
Copy link
Collaborator

albgar commented Dec 6, 2020

Just yesterday I was testing the new ansible-galaxy siesta role on a clean VM, and I got a couple of errors:

  • (on Ubuntu 20) some failures in apt-get. The solution I found was to add the equivalent of an 'apt update' at the beginning of my apt task. Now I see that the Quantum Mobile has an "ansible_prerequisites" role that does that, among other things. So this might be needed for my VM, but not for the QM. In any case, I do not think it hurts, so I will put it in a forthcoming merge request.

  • The Siesta tests were failing due to not finding a shared library. This is because the ldconfig was done after the tests... I have fixed it now.
    I wonder why the CI did not catch this. Does it have to do with the fact that in the molecule setup for the CI the fail-fast: false option is used?.

For the first issue, it could be that in Docker the apt problem does not happen...

Both things are fixed now. I am going to make another pull request.

@chrisjsewell
Copy link
Member Author

Does it have to do with the fact that in the molecule setup for the CI the fail-fast: false option is used?.

No this just means that if it fails for one platform (e.g. Ubuntu 16) it will not immediately stop running the parallel tests for the other platforms

@chrisjsewell
Copy link
Member Author

chrisjsewell commented Dec 6, 2020

The solution I found was to add the equivalent of an 'apt update' at the beginning of my apt task

you don't need a separate task, you can just add to the first apt task:

  apt:
    update_cache: yes
    cache_valid_time: 86400
    ...

i.e. update before installing, but only if the cache has not been updated in the last day

@chrisjsewell
Copy link
Member Author

@albgar are you doing the PR now/soon?
Because I'm very close to starting a new QM build 😄

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