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

✨ NEW: Add AiiDA v2.X support for eiger #80

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

mbercx
Copy link
Member

@mbercx mbercx commented Apr 20, 2023

Just updated the Eiger computer and code setup files for a bunch of QE codes. Tested in the field, except for the configuration. Still a draft PR for this reason, as well as the fact that we should probably have a separate configuration directory for v2.X computers. For codes I think it's a good idea to have different folders for the different types, which I already integrated here.

@yakutovicha what do you think? From an AiiDAlab infrastructure point of view, how would you provide support for v1 & v2?

Copy link
Contributor

@yakutovicha yakutovicha left a comment

Choose a reason for hiding this comment

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

Hi @mbercx, and thanks for the PR 👍 .

I have a few suggestions, and comments:

  1. In the current setup, we still accept codes and computers in the "old" AiiDA-1.x style. They get automatically converted into AiiDA-2.x style by the script. See the output here.
  2. I see you've put codes setup into codes-installed. At the moment, this won't work as the parser is not reading that folder.

Overall, I would be happy if we could convert the "source" of codes/computers to AiiDA-2.x. But then we need to update the migration script to do 2.x -> 1.x

Comment on lines +5 to +6
transport: core.ssh
scheduler: core.slurm
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is needed right now. We have an automated conversion tool that does that.

Comment on lines -13 to -14
#SBATCH --partition=normal
#SBATCH --account=mr0
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of deleting those lines, I would rather create a new computer named multicore

Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove "eiger" from the name. I think, the path already suggests that this is for eiger.

Copy link
Member Author

Choose a reason for hiding this comment

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

For sure, thanks! Still had to clean that up, this was just me testing my local files.

@yakutovicha
Copy link
Contributor

Still a draft PR for this reason, as well as the fact that we should probably have a separate configuration directory for v2.X computers.

We discussed that some time ago and agreed to provide a migration tool instead.

Pinging @unkcpz in case he has anything to add here.

@mbercx
Copy link
Member Author

mbercx commented Apr 28, 2023

Ah, sorry, was still also in a draft state because I was playing around with it. Basically I constantly have to create new projects and wanted to stop copying my YAML files and see if I can get a good flexible setup via the code registry instead.

We discussed that some time ago and agreed to provide a migration tool instead.

You mean for AiiDAlab, right? But don't we also just want "regular" users to use the registry?

@unkcpz
Copy link
Member

unkcpz commented Apr 28, 2023

You mean for AiiDAlab, right? But don't we also just want "regular" users to use the registry?

The migration script is provided in this repository to give two versions of JSON database for AiiDAlab use cases. @mbercx you are correct about this. I think since we gradually at the last stage of moving everything to v2, for this repo it makes more sense to having yaml file in v2 style and generate v1 format backward.

want "regular" users to use the registry?

This becomes a barrier to use even. how many users on Earth are actually using it to set up? This is also a very key point for usability. The current structure didn't provide convenience but more confusion in my sense.

For this PR itself, I think @mbercx you can just keep using the v1 format as other setups. In AiiDAlab then it can be founded and "potentially" be used.

P.S I have a very clear developing path how to support the template for code/computer setup and also having it used in AiiDAlab (which should be even more flexible and easy to support). It requires at least 0.5 person month of work approximately, let's see when I can find time to start that.

@mbercx
Copy link
Member Author

mbercx commented Apr 29, 2023

It indeed isn't practical. ^^ I was thinking of integrating the registry with the aiida-project tool I was playing around with, see:

aiidateam/aiida-project#1

Of course there I could also use this migration script. We should perhaps discuss how to integrate these efforts. I agree that perhaps we should invert the migration approach though, i.e. provide the YAML files for v2 and allow the script to downgrade them.

P.S I have a very clear developing path how to support the template for code/computer setup and also having it used in AiiDAlab (which should be even more flexible and easy to support). It requires at least 0.5 person month of work approximately, let's see when I can find time to start that.

Great! Happy to discuss this. I think having an easily accessible and smooth functioning registry will help a lot with making things easier for new users within our HPC ecosystem.

work_dir: /scratch/e1000/{username}/aiida/
shebang: '#!/bin/bash'
mpirun_command: srun -n {tot_num_mpiprocs}
mpirun_command: srun -n {tot_num_mpisprocs}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mpirun_command: srun -n {tot_num_mpisprocs}
mpirun_command: srun -n {tot_num_mpiprocs}

look_for_keys: true
timeout: 60
allow_agent: true
proxy_jump: ''
Copy link
Member

Choose a reason for hiding this comment

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

It only works for me when adding this.

Suggested change
proxy_jump: ''
proxy_jump: 'ela.cscs.ch'

@superstar54
Copy link
Member

Please also add code for dos. Thanks!

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.

4 participants