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

Improved workflow sets #625

Merged
merged 10 commits into from May 13, 2024
Merged

Conversation

zhubonan
Copy link
Member

@zhubonan zhubonan commented Dec 27, 2022

Please check the applicable boxes, thank you:

I, the author consider this PR

  • ready to be reviewed and merged (as soon as tests have passed)
  • work in progress (early feedback welcome)

Interactions with issues / other PRs

type "#" followed by search words to find issues / PRs

supersedes:
#516
blocks:

is blocked by:

None of the above but is still related to the following:

Description

Formal PR for merging the workchain stack that I use to the main code base. I call the set v2 for now in order to distinguish them with the existing ones.

A brief summary of the improvements:

VaspWorkChain

  • Accept ldau_mapping to make setting up LDA+U calculations eaiser.
  • Accept kpoints spacing to make setting up KPOINTS mesh eaiser, although the units is in the CASTEP convention, e.g. 2pi * A^-1 instead of A^-1 that VASP uses.

VaspConvergeWorkChain

This workchain is simplified from the origin one, and the calculations are all carried out in parallel.

VaspBandsWorkChain

This workchain includes an optional relaxation phase. Multiple path scehemes are supported via sumo.
An alternative workchain VaspHybridBandsWorkChain is designed for performing band structure using hybrid functionals (can be used for standard GGA as well). This workchain uses the "zero-weighted" kpoints approaches and split the band paths into multiple segments.

VaspRelaxWorkChain

This workchain is similar to the original relaxation workchain, but the settings are supplied in a single Dict node under the relax_settings port (and validated before workchain submission).
Additional checks are also performed for the final singlepoint calculation to see if it is indeed relaxed (VASP can sometime ended up in a different local minimum in this final singlepoint calculation, resulting inconsistent energies).
It also allows the parameters of the final static calculation to be overriden, say if one wants to use a different incar tag such as ISMEAR.

OptionContainer

Is the extendable class powering options to be specified as Dict. It allows keys to be pre-declared with defaults and also allows automatic conversion and validation integrated with the spec.input in the define part of the WorkChain.

VaspInputSet

A class to load and apply existing input sets to a ProcessBuilder and simply tedious works for mapping magnetic moments and passing LDA+U keys and LMAXMIX etc.

Dryrun

Allow dryrun calculation using VASP executable installed in the local machine. This runs VASP for up to several seconds or until a certain line is printed in the outputs. The OUTCAR is then parsed to check the lengths of different dimensions of the wavefunction, e.g. number of kpoints, bands and wave vectors, which can be used to determine the optium parallelisation settings.

TODO

  • Write documentation
  • Include tests
  • Point v1 entrypoints to the existing workchains, and issue deprecation warnings when applicable.

Comments are wellcome @atztogo @espenfl @JPchico

These workflows have improved logic and interfaces compared to the
existing ones.
Also added ways to setup calculations using existing inputs sets and
allow calculations/workflows to be dryrun locally for error checking
and parameters tweaking (for parallelisation).

Temporary naming these workflows with prefix `v2` in the entrypoint
names. The existing workflows can be renamed as `v1` and deprecated
later if needed.
@zhubonan
Copy link
Member Author

I got lots of no-member complains from pylint with tox, any idea why? @espenfl

aiida_vasp/calcs/neb.py:401:43: E1101: Module 'aiida.orm' has no 'StructureData' member (no-member)

running pre-commit run --all does not complain about it though.

@espenfl espenfl changed the title Pr improved workflow sets Draft: Pr improved workflow sets Dec 28, 2022
@espenfl
Copy link
Collaborator

espenfl commented Dec 28, 2022

I got lots of no-member complains from pylint with tox, any idea why? @espenfl

That is strange. What about running tox locally? Same?

@espenfl
Copy link
Collaborator

espenfl commented Dec 28, 2022

Great additions and extensions @zhubonan. Thanks for adding them. I will add a few specific comments that we can continue to discuss.

@espenfl
Copy link
Collaborator

espenfl commented Dec 28, 2022

VaspConvergeWorkChain

I think we should try to make these not dependent on VASP as most of these should be equally relevant for QE or Abinit. In fact, we should maybe try to coordinate further efforts on these type of general workchains with the QE and Abinit gang to make them more unified. As it stands, I think these can be rather easily generalized as is as well to open up for this in the future.

@espenfl
Copy link
Collaborator

espenfl commented Dec 28, 2022

Dryrun

Yes, this is certainly a good option. I think, we can also do remote dryruns now as we can monitor a remote running calc with the recent additions in aiida core. I have not tried that, but maybe we should look into that, so that also a remote code can be used here.

@espenfl
Copy link
Collaborator

espenfl commented Dec 28, 2022

And a general comment, what is the take on the release with respect to these additions? Here we are most likely breaking a few workchain interfaces, so would maybe just make sense to add them as the necessary docs updates would not be done in at least a week or so. Your opinions on this would be great. I suspect most of us will not work much until next week so we should also consider that.

@zhubonan
Copy link
Member Author

That is strange. What about running tox locally? Same?

It also shows up, the errors are pasted below.

pre-commit inst-nodeps: /home/bonan/aiida_envs/aiida-2.0-dev/aiida-vasp/.tox/.tmp/package/1/aiida-vasp-2.1.1.tar.gz
WARNING: Discarding $PYTHONPATH from environment, to override specify PYTHONPATH in 'passenv' in your configuration.
pre-commit installed: aiida-core==2.2.1,aiida-export-migration-tests==0.9.0,aiida-vasp==2.1.1,aio-pika==6.8.1,aiormq==3.3.1,alabaster==0.7.12,alembic==1.9.1,archive-path==0.4.2,ase==3.22.1,asn1crypto==1.5.1,astroid==2.8.6,async-generator==1.10,attrs==22.2.0,Babel==2.11.0,backcall==0.2.0,bcrypt==4.0.1,bzscripts==0.1.0,certifi==2022.12.7,cffi==1.15.1,cfgv==3.3.1,charset-normalizer==2.1.1,circus==0.18.0,click==8.1.3,click-config-file==0.6.0,click-spinner==0.1.10,configobj==5.0.6,contourpy==1.0.6,coverage==6.5.0,cryptography==38.0.4,cycler==0.11.0,decorator==5.1.1,deprecation==2.1.0,disk-objectstore==0.6.0,distlib==0.3.6,docutils==0.16,emmet-core==0.39.6,exceptiongroup==1.1.0,filelock==3.8.2,fonttools==4.38.0,future==0.18.2,graphviz==0.20.1,greenlet==2.0.1,identify==2.5.11,idna==3.4,imagesize==1.4.1,importlib-metadata==4.13.0,importlib-resources==5.10.1,iniconfig==1.1.1,ipython==7.34.0,isort==5.11.4,jedi==0.18.2,Jinja2==3.1.2,jsonschema==3.2.0,kiwipy==0.7.7,kiwisolver==1.4.4,latexcodec==2.0.1,lazy-object-proxy==1.8.0,lxml==4.9.2,Mako==1.2.4,MarkupSafe==2.1.1,matplotlib==3.6.2,matplotlib-inline==0.1.6,mccabe==0.6.1,monty==2022.9.9,mp-api==0.30.5,mpmath==1.2.1,msgpack==1.0.4,multidict==6.0.4,mypy==0.991,mypy-extensions==0.4.3,nest-asyncio==1.5.6,networkx==2.8.8,nodeenv==1.7.0,numpy==1.24.1,packaging==20.3,palettable==3.3.0,pamqp==2.3.0,pandas==1.5.2,paramiko==2.12.0,parsevasp==3.1.0,parso==0.8.3,pexpect==4.8.0,pg8000==1.29.4,pgsu==0.2.3,pgtest==1.3.2,pickleshare==0.7.5,Pillow==9.3.0,platformdirs==2.6.0,plotly==5.11.0,pluggy==1.0.0,plumpy==0.21.3,pre-commit==2.21.0,prompt-toolkit==3.0.36,psutil==5.9.4,psycopg2-binary==2.9.5,ptyprocess==0.7.0,py==1.11.0,py-cpuinfo==9.0.0,pybtex==0.24.0,PyCifRW==4.4.5,pycparser==2.21,pydantic==1.10.2,Pygments==2.13.0,pylint==2.11.1,pylint_aiida==0.1.1,pymatgen==2022.11.7,Pympler==0.9,PyMySQL==0.9.3,PyNaCl==1.5.0,pyparsing==3.0.9,pyrsistent==0.19.2,pytest==7.2.0,pytest-asyncio==0.16.0,pytest-benchmark==4.0.0,pytest-cov==2.10.1,pytest-datadir==1.4.1,pytest-regressions==2.4.1,pytest-rerunfailures==9.1.1,pytest-timeout==1.4.2,python-dateutil==2.8.2,pytray==0.3.4,pytz==2021.3,PyYAML==6.0,pyzmq==24.0.1,requests==2.28.1,ruamel.yaml==0.17.21,ruamel.yaml.clib==0.2.7,scipy==1.9.3,scramp==1.4.4,seekpath==1.9.7,shortuuid==1.0.11,six==1.16.0,snowballstemmer==2.2.0,spglib==2.0.2,Sphinx==4.5.0,sphinxcontrib-applehelp==1.0.2,sphinxcontrib-devhelp==1.0.2,sphinxcontrib-htmlhelp==2.0.0,sphinxcontrib-jsmath==1.0.1,sphinxcontrib-qthelp==1.0.3,sphinxcontrib-serializinghtml==1.1.5,SQLAlchemy==1.4.45,SQLAlchemy-Utils==0.37.9,sqlalchemy2-stubs==0.0.2a31,sympy==1.11.1,tabulate==0.8.10,tenacity==8.1.0,toml==0.10.2,tomli==2.0.1,tornado==6.2,tox==3.28.0,tqdm==4.64.1,traitlets==5.8.0,types-PyYAML==6.0.12.2,typing_extensions==4.4.0,uncertainties==3.1.7,upf-to-json==0.9.5,urllib3==1.26.13,virtualenv==20.17.1,wcwidth==0.2.5,Werkzeug==2.1.2,wrapt==1.13.3,yarl==1.8.2,zipp==3.11.0
pre-commit run-test-pre: PYTHONHASHSEED='2162387008'
pre-commit run-test: commands[0] | bash -ec 'pre-commit run --all-files || ( git diff; git status; exit 1; )'
trim trailing whitespace.....................................................Passed
fix end of files.............................................................Passed
check yaml...................................................................Passed
check json...............................................(no files to check)Skipped
check toml...................................................................Passed
check for added large files..................................................Passed
fix double quoted strings....................................................Passed
forbid new submodules....................................(no files to check)Skipped
yapf.........................................................................Passed
isort........................................................................Passed
flynt........................................................................Passed
Check .rst files with the same linter used by pypi.org.......................Passed
pylint.......................................................................Failed
- hook id: pylint
- exit code: 2

-------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 9.94/10, +0.06)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

PYLINTHOME is now '/home/bonan/.cache/pylint' but obsolescent '/home/bonan/.pylint.d' is found; you can safely remove the latter

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

************* Module aiida_vasp.workchains.relax
aiida_vasp/workchains/relax.py:15:0: E0611: No name 'CalcJobNode' in module 'aiida.orm' (no-name-in-module)
aiida_vasp/workchains/relax.py:15:0: E0611: No name 'StructureData' in module 'aiida.orm' (no-name-in-module)

------------------------------------------------------------------
Your code has been rated at 9.90/10 (previous run: 9.90/10, +0.00)

************* Module aiida_vasp.workchains.v2.relax
aiida_vasp/workchains/v2/relax.py:100:23: E1101: Module 'aiida.orm' has no 'Bool' member (no-member)
aiida_vasp/workchains/v2/relax.py:220:66: E1101: Module 'aiida.orm' has no 'Bool' member (no-member)
aiida_vasp/workchains/v2/relax.py:476:12: E1101: Module 'aiida.orm' has no 'StructureData' member (no-member)
aiida_vasp/workchains/v2/relax.py:480:21: E1101: Module 'aiida.orm' has no 'CalcJobNode' member (no-member)
aiida_vasp/workchains/v2/relax.py:481:21: E1101: Module 'aiida.orm' has no 'StructureData' member (no-member)
aiida_vasp/workchains/v2/relax.py:693:27: E1101: Module 'aiida.orm' has no 'WorkChainNode' member (no-member)
aiida_vasp/workchains/v2/relax.py:698:24: E1101: Module 'aiida.orm' has no 'WorkChainNode' member (no-member)
aiida_vasp/workchains/v2/relax.py:705:24: E1101: Module 'aiida.orm' has no 'WorkChainNode' member (no-member)
aiida_vasp/workchains/v2/relax.py:714:27: E1101: Module 'aiida.orm' has no 'CalcJobNode' member (no-member)
aiida_vasp/workchains/v2/relax.py:883:16: E1101: Module 'aiida.orm' has no 'StructureData' member (no-member)
************* Module aiida_vasp.utils.fixtures.calcs
aiida_vasp/utils/fixtures/calcs.py:47:4: E0611: No name 'CalcJobNode' in module 'aiida.orm' (no-name-in-module)
aiida_vasp/utils/fixtures/calcs.py:47:4: E0611: No name 'FolderData' in module 'aiida.orm' (no-name-in-module)
aiida_vasp/utils/fixtures/calcs.py:88:4: E0611: No name 'CalcJobNode' in module 'aiida.orm' (no-name-in-module)
aiida_vasp/utils/fixtures/calcs.py:88:4: E0611: No name 'FolderData' in module 'aiida.orm' (no-name-in-module)

------------------------------------------------------------------
Your code has been rated at 9.43/10 (previous run: 9.43/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

************* Module aiida_vasp.workchains.v2.bands
aiida_vasp/workchains/v2/bands.py:11:0: E0401: Unable to import 'aiida_user_addons.process.transform' (import-error)
aiida_vasp/workchains/v2/bands.py:13:0: E0401: Unable to import 'sumo.symmetry.kpoints' (import-error)

------------------------------------------------------------------
Your code has been rated at 9.87/10 (previous run: 9.87/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)


--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

************* Module aiida_vasp.utils.fixtures.environment
aiida_vasp/utils/fixtures/environment.py:20:0: E0611: No name 'CalculationNode' in module 'aiida.orm' (no-name-in-module)

------------------------------------------------------------------
Your code has been rated at 9.93/10 (previous run: 9.93/10, +0.00)

WARNING: terminal is not fully functional

@zhubonan
Copy link
Member Author

I think we should try to make these not dependent on VASP as most of these should be equally relevant for QE or Abinit.

I will looking into this, although I think our priority is to get it working for VASP. I should be easy to generalise to other codes by refectoring the methods (update cut off, k spacing and getting energies/forces/stresses).
However, it is still an open question on how the dependency. Ideally one does not want to have the plugins be inter-dependent on each other.

@zhubonan
Copy link
Member Author

Yes, this is certainly a good option. I think, we can also do remote dryruns now as we can monitor a remote running calc with the recent additions in aiida core. I have not tried that, but maybe we should look into that, so that also a remote code can be used here.

I see. I have not tried that either. The dryrun here is just to run locally before submitting the workchain (get the number of kpoints for example). It is also possible to add this into the workchain, e.g. have some kind of auto-parallel, but I hvae not written a working logic for setting KPAR/NCOREs, as it also depends on the hardware of the remote cluster.

what is the take on the release with respect to these additions? Here we are most likely breaking a few workchain interfaces, so would maybe just make sense to add them as the necessary docs updates would not be done in at least a week or so.

What I intended is to have these additions as "non-breaking" changes, at least for now. Feel free to have the new release ("3.x"?) before/after the PR is merged. What I have in mind is to have:

  • The new workflows in this PR under the "vasp.v2" entrypoints, and the current workchains under the "vasp.v1" entrypoint.
  • The vasp entrypoints still points to the "vasp.v1" classes (current workchains) until the next major release. The current workchain should work the same way as before until the this next major release.
  • In the next major release, we change the (default) "vasp.xxx" entrypoints to point the workflows in this PR, and keep the v1 and v2 sub namespaces. We can still keep the "vasp.v1" workchains for a while.

@zhubonan zhubonan self-assigned this Jan 13, 2023
@zhubonan
Copy link
Member Author

zhubonan commented Apr 22, 2024

This PR needs to be updated to reflect changes in the aiida-user-addons' code (should be minor) as well as those in the aiida-vasp.

I think it is good to have it merged soon.

The missing things are:

  • Documentation on how to use the new workflows
  • Test for the workflows
  • Check if there are breaking changes

Copy link

codecov bot commented Apr 26, 2024

Codecov Report

Attention: Patch coverage is 17.14507% with 2136 lines in your changes are missing coverage. Please review.

Project coverage is 53.62%. Comparing base (cf00bb8) to head (9b33c90).

Files Patch % Lines
src/aiida_vasp/workchains/v2/relax.py 15.30% 382 Missing ⚠️
src/aiida_vasp/workchains/v2/bands.py 13.66% 373 Missing ⚠️
...aiida_vasp/workchains/v2/common/builder_updater.py 0.00% 330 Missing ⚠️
src/aiida_vasp/workchains/v2/common/transform.py 16.24% 196 Missing ⚠️
src/aiida_vasp/workchains/v2/converge.py 14.03% 190 Missing ⚠️
src/aiida_vasp/workchains/v2/common/dryrun.py 0.00% 170 Missing ⚠️
src/aiida_vasp/workchains/v2/vasp.py 17.74% 116 Missing ⚠️
src/aiida_vasp/commands/dryrun_vasp.py 0.00% 107 Missing ⚠️
src/aiida_vasp/workchains/v2/common/opthold.py 36.37% 105 Missing ⚠️
src/aiida_vasp/workchains/v2/common/dictwrap.py 0.00% 59 Missing ⚠️
... and 6 more
Additional details and impacted files
@@             Coverage Diff              @@
##           develop     #625       +/-   ##
============================================
- Coverage    65.44%   53.62%   -11.82%     
============================================
  Files           58       74       +16     
  Lines         7325     9903     +2578     
============================================
+ Hits          4793     5309      +516     
- Misses        2532     4594     +2062     
Flag Coverage Δ
pytests 53.62% <17.15%> (-11.82%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@zhubonan
Copy link
Member Author

@atztogo @JPchico Could you please take a look at this PR? I think it is ready to be merged. We can add more tests later for the new workchain stack later.

@zhubonan zhubonan requested a review from JPchico May 12, 2024 09:54
Copy link
Collaborator

@atztogo atztogo left a comment

Choose a reason for hiding this comment

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

LGTM.

@zhubonan zhubonan changed the title Draft: Pr improved workflow sets Improved workflow sets May 13, 2024
@zhubonan zhubonan merged commit 08a952f into aiida-vasp:develop May 13, 2024
5 of 7 checks passed
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.

None yet

3 participants