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

Clean up test files: VASP inputs #3674

Merged
merged 26 commits into from
Mar 8, 2024

Conversation

DanielYang59
Copy link
Contributor

@DanielYang59 DanielYang59 commented Mar 7, 2024

Clean up test files: VASP inputs

The second episode of cleaning up test files: VASP inputs, relocate VASP input files to a dedicated dir (vasp/inputs), following #3653.

TODOs

@DanielYang59
Copy link
Contributor Author

Meanwhile I noticed some input files are compressed for example:

def test_standard_with_energy_range_from_vasprun(self):
# test standard_with_energy_range_from_vasprun
lobsterin_comp = Lobsterin.standard_calculations_from_vasp_files(
f"{TEST_FILES_DIR}/POSCAR.C2.gz",
f"{TEST_FILES_DIR}/INCAR.C2.gz",
f"{TEST_FILES_DIR}/POTCAR.C2.gz",
f"{VASP_OUT_DIR}/vasprun.C2.xml.gz",
option="standard_with_energy_range_from_vasprun",
)

I didn't see much point in compressing these small and "human-oriented" inputs files (INCAR/POSCAR/KPOINTS) so I might save them decompressed.

@janosh janosh added tests Issues with or changes to the pymatgen test suite housekeeping Moving around or cleaning up old code/files vasp Vienna Ab initio Simulation Package dx Developer experience labels Mar 7, 2024
@DanielYang59

This comment was marked as outdated.

@DanielYang59 DanielYang59 marked this pull request as ready for review March 8, 2024 11:38
@DanielYang59
Copy link
Contributor Author

Should be good for review now @janosh. Thanks a ton for your time and let me know if you want any more changes.

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.

thanks, great work as usual! 👍

@janosh janosh merged commit 1dadefd into materialsproject:master Mar 8, 2024
22 checks passed
@DanielYang59
Copy link
Contributor Author

Thanks for reviewing @janosh . Meanwhile I might need some of your advice on the third episode, where I plan to collect all those randomly scattered VASP directories in test files together. As such it seems we would have two options:

  1. Lay all directories side by side with inputs and outputs:
vasp
├── inputs
├── outputs
├── vasp_dir_0
└── vasp_dir_1

This could prevent the test directory being too deeply nested, but would potentially bury the important inputs and outputs if there are too many.

  1. Otherwise we could have another directory (I would need a better name than vasp_project):
vasp
├── inputs
├── outputs
└── vasp_project
    ├── vasp_dir_0
    └── vasp_dir_1

I would personally prefer the 2nd. What do you think?

@janosh
Copy link
Member

janosh commented Mar 8, 2024

i think both options are fine. if we go with option 2, fixtures is one possible name for vasp_project as it's commonly used for directories with reference files so will be easily understood by many people. it's a slight misuse though as basically then the whole tests/files dir could be called tests/fixtures instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dx Developer experience housekeeping Moving around or cleaning up old code/files 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.

3 participants