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

Add unit test for io.vasp.help #4020

Merged
merged 28 commits into from
Sep 4, 2024

Conversation

DanielYang59
Copy link
Contributor

@DanielYang59 DanielYang59 commented Aug 27, 2024

Summary

  • Add unit test for io.vasp.help
  • Bump python requirement to 3.10+ in README, thanks @QuantumChemist for pointing out (re pattern: py(?:thon)?\s?3\.?9)
  • Reduce timeout for requests to 60 seconds, 600 seconds might be too impractical

Not planned (for this PR?)

  • test_get_incar_tags also check incar_parameters.json file
  • Update incar_parameters.jsonfile

@DanielYang59 DanielYang59 marked this pull request as ready for review August 27, 2024 11:44
Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

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

nice work! always good to have more tests! 👍

tests/io/vasp/test_help.py Outdated Show resolved Hide resolved

def test_get_incar_tags(self):
incar_tags = VaspDoc.get_incar_tags()
assert incar_tags
Copy link
Member

Choose a reason for hiding this comment

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

can we assert incar_tags type and/or actual value?

Copy link
Contributor Author

@DanielYang59 DanielYang59 Aug 28, 2024

Choose a reason for hiding this comment

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

Good point actually. I didn't know what to assert in this first place because these INCAR tags are fetched from VASP wiki and might be prone to change (so if we assert some INCAR tags that are in the list right now, they might be removed at some point in the future, though sounds very unlikely to happen frequently/soon).

Update: marked as draft, I might use this to check the incar_parameters.json file

Copy link
Contributor Author

@DanielYang59 DanielYang59 Aug 28, 2024

Choose a reason for hiding this comment

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

Can I have a quick comment on the following @jansoh? Thanks in advance.

2ede975 seems to reveal quite some difference between INCAR tags from VASP wiki and incar_parameters.json file. I'm not sure if VASP wiki is the complete set of INCAR tags, and what about VASP versioning (does VASP wiki only display INCAR tags for the latest version of VASP)?

Tags in json file but not in VASP wiki (remove them from json file?) (are they just absent from latest version of VASP?):

'ML_FF_SION2_MB', 'ML_FF_AFILT2_MB', 'LCOMPAT', 'ML_FF_MB_MB', 'ML_FF_IBROAD2_MB', 'HFSCREENC', 'ML_FF_SIGW0_MB', 'ML_FF_ISTART', 'MIXFIRST', 'MAGPOS', 'ML_FF_ICUT1_MB', 'ML_FF_IAFILT2_MB', 'ML_FF_CSIG', 'ML_FF_W1_MB', 'ML_FF_ISAMPLE', 'DARWINV', 'ML_FF_SIGV0_MB', 'ML_FF_RCUT1_MB', 'ML_FF_ISOAP1_MB', 'LGAUGE', 'ML_FF_ISOAP2_MB', 'ML_FF_IREG_MB', 'ML_FF_CTIFOR', 'ML_FF_MSPL1_MB', 'ML_FF_CSLOPE', 'ML_FF_CSF', 'ML_FF_LAFILT2_MB', 'ML_FF_LCRITERIA', 'ML_FF_MSPL2_MB', 'ML_FF_NDIM_SCALAPACK', 'ISPIND', 'LMUSIC', 'ML_FF_LEATOM_MB', 'ML_FF_ISCALE_TOTEN_MB', 'LUSEW', 'ML_FF_NATOM_COUPLED_MB', 'ML_FF_IWEIGHT', 'LSYMGRAD', 'SHIFTRED', 'ML_FF_IBROAD1_MB', 'NDAV', 'ML_FF_ICOUPLE_MB', 'ML_FF_MCONF', 'LNICSALL', 'STM', 'ML_FF_LNORM1_MB', 'ML_FF_MHIS', 'ML_FF_W2_MB', 'ML_FF_LBASIS_DISCARD', 'ML_FF_LNORM2_MB', 'ML_FF_LMAX2_MB', 'ML_FF_RCOUPLE_MB', 'ML_FF_LMLFF', 'LSOL', 'ENMAX', 'LHFONE', 'ML_FF_LCONF_DISCARD', 'LORBITALREAL', 'DARWINR', 'ML_FF_WTSIF', 'ML_FF_IERR', 'ML_FF_NR2_MB', 'ML_FF_LMLMB', 'LFERMIGW', 'ML_FF_CDOUB', 'ML_FF_NMDINT', 'ML_FF_LCOUPLE_MB', 'ML_FF_WTOTEN', 'ML_FF_MRB1_MB', 'ML_FF_LHEAT_MB', 'ML_FF_NWRITE', 'LMAGBLOCH', 'ML_FF_MCONF_NEW', 'ML_FF_SION1_MB', 'LTCTE', 'SELFENERGY', 'ML_FF_NR1_MB', 'MAGDIPOLOUT', 'LFXHEG', 'ML_FF_NHYP2_MB', 'ML_FF_MRB2_MB', 'LTETE', 'ML_FF_RCUT2_MB', 'LDOWNSAMPLE', 'ML_FF_ICUT2_MB', 'LVEL', 'LFXCEPS', 'NUCIND', 'ML_FF_NHYP1_MB', 'ML_FF_EATOM', 'LMETAGGA', 'ML_FF_WTIFOR', 'ENCUTLF', 'ORBITALMAG'

Tags in VASP wiki but not in json file (add them to json file):

'LIBMBD_MBD_BETA', 'ML_EPS_REG', 'GAMMA_VDW', 'ML_SIGV0', 'VDW_ALPHA', 'LVGVCALC', 'ML_WTOTEN', 'LWAVEH5', 'ML_LAFILT2', 'ML_RCOUPLE', 'LTEMPER', 'LPHON_POLAR', 'LTSSURF', 'NTEMPER', 'ML_MODE', 'LALL_IN_ONE', 'ML_MB_MIN', 'LVDW_ONECELL', 'CUTOFF_TYPE', 'ML_EPS_LOW', 'ML_NMDINT', 'SMBJ', 'ML_MB', 'LVACPOTAV', 'ML_LHEAT', 'ML_MRB2', 'ML_LERR', 'CUTOFF_MU', 'ML_RCUT2', 'LSMP2LT', 'LIBXC1_Pn', 'NOMEGA_DUMP', 'LIBMBD_ALPHA', 'PHON_G_CUTOFF', 'PHON_BORN_CHARGES', 'Construction:NBSEBLOCKV', 'ML_OUTPUT_MODE', 'ML_IAFILT2', 'LKPOINTS_WAN', 'ML_CTIFOR', 'ALPHA_VDW', 'BPARAM', 'IRC_MAXSTEP', 'LH5', 'FOCKCORR', 'LIBMBD_R0AU', 'ML_CSLOPE', 'LWEIGHTED', 'PHON_SIGMA', 'LSCDM', 'Construction:LPARDH5', 'ML_IWEIGHT', 'LTBOUNDLIBXC', 'IVDW_NL', 'IRC_STOP', 'LIBMBD_K_GRID_SHIFT', 'ML_ISCALE_TOTEN', 'ML_NRANK_SPARSDES', 'BEXT', 'NHC_NCHAINS', 'ML_IERR', 'ML_ICRITERIA', 'LIBXC2', 'ML_NHYP', 'CUTOFF_SIGMA', 'ML_WTIFOR', 'PHON_NWRITE', 'LIBMBD_TS_SR', 'LIBMBD_TS_D', 'SCALEE', 'LIBMBD_MBD_A', 'BANDGAP', 'WANNIER90_WIN', 'NHC_NS', 'ML_RCUT1', 'ML_CDOUB', 'LFINITE_TEMPERATURE', 'ML_ICOUPLE', 'LIBMBD_K_GRID', 'LWRITE_SPN', 'NHC_NRESPA', 'VDW_C6AU', 'ML_SCLC_CTIFOR', 'LIBMBD_N_OMEGA_GRID', 'CMBJE', 'LSPIN_VDW', 'NUM_WANN', 'ML_W1', 'IALL_IN_ONE', 'XC', 'ML_AFILT2', 'LFOCKACE', 'Construction:NBSEBLOCKO', 'LIBMBD_PARALLEL_MODE', 'ML_LBASIS_DISCARD', 'PHON_DIELECTRIC', 'XC_C', 'LPHON_READ_FORCE_CONSTANTS', 'ML_LEATOM', 'BSEELECTRON', 'HFALPHA', 'ESTOP', 'ML_WTSIF', 'ML_MCONF', 'PHON_DOS', 'VCA', 'ML_LMLFF', 'CPARAM', 'LPHON_DISPERSION', 'VDW_R0AU', 'ML_MRB1', 'NATURALO', 'ZAB_VDW', 'ML_LSPARSDES', 'NBANDS_WAVE', 'WRT_POTENTIAL', 'ML_OUTBLOCK', 'ALDAX', 'NRMM', 'RSMBJ', 'LIBMBD_C6AU', 'ML_LMAX2', 'BSEHOLE', 'ML_RDES_SPARSDES', 'VCAIMAGES', 'NELMGW', 'NSTORB', 'NMAXFOCKAE_and_LMAXFOCKAE', 'EFERMI', 'ML_LCOUPLE', 'Profiling', 'SCISSOR', 'BSEPREC', 'LMP2LT', 'LIBMBD_METHOD', 'LDISENTANGLED', 'NBANDSEXACT', 'ML_ISTART', 'ML_IALGO_LINREG', 'AMGGAC', 'ML_EATOM_REF', 'AMGGAX', 'ML_IREG', 'ML_SION2', 'LVDWSCS', 'LCHARGH5', 'IRC_DIRECTION', 'ML_LFAST', 'STOP_ON', 'ML_CSIG', 'LIBXC2_Pn', 'KPOINTS_OPT_NKBATCH', 'ML_SION1', 'IRC_VNORM0', 'IRC_DELTA0', 'ML_DESC_TYPE', 'ML_NATOM_COUPLED', 'LIBMBD_XC', 'ML_CX', 'ML_SIGW0', 'LSCK', 'Construction:EFOR', 'PHON_NEDOS', 'NCORE_IN_IMAGE1', 'LUSENCCL', 'LSCALER0', 'VACPOTFLAT', 'LIBXC1', 'IRC_MINSTEP', 'ML_MCONF_NEW', 'ML_MHIS', 'LIBMBD_VDW_PARAMS_KIND', 'LKPOINTS_OPT', 'LVGVAPPL'

Might be good for me to write a script to automate such things if so, there are just too many tags to manually update...

Copy link
Member

Choose a reason for hiding this comment

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

i assume the VASP docs cover both the latest and earlier versions. if the tags are no longer documented, they may have been deprecated a while ago. still probably makes sense for incar_parameters.json to be permissive, i.e. allow older tags that have been deprecated (though ideally with a warning to the user when deprecated tags are validated). maybe @esoteric-ephemera knows more

Copy link
Contributor

Choose a reason for hiding this comment

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

The current VASP wiki INCAR tags contain old and new tags, e.g. is IALGO a deprecated version of ALGO. But I can't say why there are some tags in the json that don't show up in the wiki.

Copy link
Contributor Author

@DanielYang59 DanielYang59 Aug 28, 2024

Choose a reason for hiding this comment

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

Thanks for the comment @QuantumChemist, looks like we got a small issue with the get_incar_tags method:

def get_incar_tags(cls) -> list[str]:
"""Get a list of all INCAR tags from the VASP wiki."""
tags = []
for page in (
"https://www.vasp.at/wiki/index.php/Category:INCAR",
"https://www.vasp.at/wiki/index.php?title=Category:INCAR&pagefrom=ML+FF+LCONF+DISCARD#mw-pages",
):

Looks like the INCAR list page from VASP wiki has changed probably, some pages are missing. I would update this now.

The original json file was added in 2019 #1577, perhaps some tags are deprecated and removed since?

Copy link
Contributor

@QuantumChemist QuantumChemist Aug 28, 2024

Choose a reason for hiding this comment

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

sorry I just overread LREAL. It's in the wiki INCAR tag list.

tests/io/vasp/test_help.py Outdated Show resolved Hide resolved
@janosh janosh added tests Issues with or changes to the pymatgen test suite io Input/output functionality vasp Vienna Ab initio Simulation Package labels Aug 27, 2024
@DanielYang59 DanielYang59 requested a review from janosh August 28, 2024 01:33
@DanielYang59 DanielYang59 marked this pull request as draft August 28, 2024 06:05
@esoteric-ephemera
Copy link
Contributor

esoteric-ephemera commented Aug 29, 2024 via email

@esoteric-ephemera
Copy link
Contributor

esoteric-ephemera commented Sep 3, 2024

@DanielYang59 for updating VASP defaults, you can take a look at the pymatgen-io-validation add-on from @matthewkuner and myself. I think we got most of the relevant defaults there, otherwise you'll need to scrape them from the site

There are also undocumented tags which we can safely ignore. There are a set of "joke" VASP settings (like LMUSIC and IDIOT), and also developmental/debugging tags

The last category is VASP add-ons which can be compiled as external libraries to VASP, and add INCAR tags that VASP would not document. Some of these add-ons (like BEEF, DFTD4, and VTST) are common enough that it maybe warrants documenting a few in pymatgen

@DanielYang59
Copy link
Contributor Author

Hi @esoteric-ephemera great to see your response as always. Thanks a lot for the reply :)

for updating VASP defaults, you can take a look at the pymatgen-io-validation add-on from @matthewkuner and myself. I think we got most of the relevant defaults there

Perhaps there's some misunderstanding here, but I was intended to test/update the io/vasp/incar_parameters.json file which contains the known types for INCAR tags (not really default values).

There are also undocumented tags which we can safely ignore. There are a set of "joke" VASP settings (like LMUSIC and IDIOT), and also developmental/debugging tags

I didn't see those "joke" tags in VASP wiki so perhaps we could safely ignore them for now.

The last category is VASP add-ons which can be compiled as external libraries to VASP, and add INCAR tags that VASP would not document.

Great to know that. In this regard I agree with Janosh that we'd better be more permissive to include those tags in the json file which are not listed in VASP wiki for any reason.

@DanielYang59 DanielYang59 marked this pull request as ready for review September 4, 2024 03:50
@DanielYang59
Copy link
Contributor Author

@janosh Perhaps let's just test io.vasp.help for now? Sound like there's more discussion needed to check and update incar_parameters.json file (too many INCAR tags are absent from the json file).

@janosh janosh merged commit ed52258 into materialsproject:master Sep 4, 2024
43 checks passed
@DanielYang59 DanielYang59 deleted the unit-test-vasp-help branch September 4, 2024 14:07
@janosh
Copy link
Member

janosh commented Sep 4, 2024

always love to see new tests! thanks @DanielYang59 👍

@DanielYang59
Copy link
Contributor Author

No problem, thanks for reviewing and everyone for the helpful discussion :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io Input/output functionality tests Issues with or changes to the pymatgen test suite vasp Vienna Ab initio Simulation Package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants