Clean up test files: VASP outputs

[DanielYang59]

Clean up test files: VASP outputs

This is the first step of cleaning up test files: VASP outputs, relocate VASP output files to a dedicated dir (vasp/outputs) under tests dir to tidy up the files dir.

With the aim to clean up test files so that future developers could easily find and reuse existing test files whenever possible.

TODOs

• Relocate vasprun.xml c68d5c3 and 2c8eba7
• Relocate OUTCAR ee6df80
• Relocate CHGCAR 37868ce
• Relocate DOSCAR 649152a
• Relocate EIGENVAL 1bba1bf
• Relocate PROCAR 099c205
• Relocate WAVECAR 428c326
• Relocate XDATCAR 0044e49 and bd07afd
• Relocate LOCPOT 897fa39
• Relocate CONTCAR cb9fd86
• Relocate other VASP output files (DYNMAT 8c69192, ELFCAR 71964ad, IBZKPT eb33d40, WAVEDER 002225f, OSZICAR b2d9040).
• Remove duplicates a7301ba, 82c3729
• Unify test file namings a1c25f7, c4d7e6a, 0fa962d, f2e0526

Future follow up PRs

• Clean up VASP input files
• Group and clean up dedicated VASP dirs, for example tests/files/relaxation, tests/files/kpoints_opt and more.

[DanielYang59]

Do you have any comments so far before I proceed? @janosh

Test file naming

Regarding the test file naming conventions, I would like to hear about your suggestions. Taking vasprun.xml for example, I'm thinking of:

• Keep vasprun as the header so that they could be listed and grouped properly. For example bad_vasprun.xml should not be used.
• Use xml.gz as the tail to respect the file extension.

In this case, maybe either vasprun_something.xml.gz or vasprun.something.xml.gz? Which one would you suggest? Or don't change them at this moment? I would then leave a README there after this too.

Tests for borg broken

Meanwhile my relocations seem to break tests for borg, as the outputs and inputs for VASP are now not inside the same dir:

pymatgen/tests/apps/borg/test_hive.py

Lines 28 to 30 in c05931c

	entry = self.drone.assimilate(TEST_FILES_DIR)
	for param in ("hubbards", "is_hubbard", "potcar_spec", "run_type"):
	assert param in entry.parameters

Which seems to pack os.listdir():

pymatgen/pymatgen/apps/borg/hive.py

Lines 104 to 113 in c05931c

	def assimilate(self, path):
	"""Assimilate data in a directory path into a ComputedEntry object.
	Args:
	path: directory path
	Returns:
	ComputedEntry
	"""
	files = os.listdir(path)

But I'm not familiar with borg so 28c784c may not be a rational fix. Can you help me double check this?

Thanks a lot 😃 !

[janosh]

i noticed that as well. not sure why but also haven't investigated this at all yet. my suspicion was the windows runner just use slower hardware but maybe that's wrong and it's something in our test setup.

either way it makes sense to occasionally update the .pytest-durations.json file that determines how the workload is optimally split across concurrent test runners so they all take roughly equally long. and as the code changes, that file needs to be updated once in a while

[DanielYang59]

my suspicion was the windows runner just use slower hardware but maybe that's wrong and it's something in our test setup.

Not sure (don't have much knowledge about Github Actions...). But I guess it has something to do with the runners themselves, as I notice there're others reporting this as well.

either way it makes sense to occasionally update the .pytest-durations.json file that determines how the workload is optimally split across concurrent test runners so they all take roughly equally long.

Well I don't think this would help much though... As currently Windows runners seem consistently slower than Ubuntu, so even we balance the runtime of both sides, the final run time would still be determined (and slowed) by Windows 😿

[DanielYang59]

Please review @janosh.

I have to keep the test file name for borg as vasprun.xml.xe.gz instead of the expected vasprun.xe.xml.gz because it is hardcoded to collect info from vasprun files of pattern vasprun.xml*:

pymatgen/pymatgen/apps/borg/hive.py

Lines 104 to 133 in b96d9e1

	def assimilate(self, path):
	"""Assimilate data in a directory path into a ComputedEntry object.
	Args:
	path: directory path
	Returns:
	ComputedEntry
	"""
	files = os.listdir(path)
	if "relax1" in files and "relax2" in files:
	filepath = glob(f"{path}/relax2/vasprun.xml*")[0]
	else:
	vasprun_files = glob(f"{path}/vasprun.xml*")
	filepath = None
	if len(vasprun_files) == 1:
	filepath = vasprun_files[0]
	elif len(vasprun_files) > 1:
	# Since multiple files are ambiguous, we will always read
	# the one that it the last one alphabetically.
	filepath = sorted(vasprun_files)[-1]
	warnings.warn(f"{len(vasprun_files)} vasprun.xml.* found. {filepath} is being parsed.")
	try:
	vasp_run = Vasprun(filepath)
	except Exception as exc:
	logger.debug(f"error in {filepath}: {exc}")
	return None
	return vasp_run.get_computed_entry(self._inc_structure, parameters=self._parameters, data=self._data)

I wouldn't touch the source code because I don't have proper knowledge of borg.

Noticed some tests are failing because pydantic and matgl are missing. Not sure about the reason though.

[janosh]

Well I don't think this would help much though... As currently Windows runners seem consistently slower than Ubuntu, so even we balance the runtime of both sides, the final run time would still be determined (and slowed) by Windows 😿

that's certainly true

I have to keep the test file name for borg as vasprun.xml.xe.gz instead of the expected vasprun.xe.xml.gz because it is hardcoded to collect info from vasprun files of pattern vasprun.xml*:

sure, makes sense

Noticed some tests are failing because pydantic and matgl are missing. Not sure about the reason though.

not sure either pydantic should not be needed at all and matgl should be installed in CI. i can try to debug tomorrow

pymatgen/setup.py

Line 81 in b96d9e1

"matgl",

[DanielYang59]

not sure either pydantic should not be needed at all and matgl should be installed in CI. i can try to debug tomorrow

pymatgen/setup.py

Line 81 in b96d9e1

"matgl",

That would be much appreciated. Also noticed tests only failed for Ubuntu not Windows.

It seems matgl is installed successfully during CI run stage Install dependencies:

Successfully installed .......  matgl-1.0.0 ......

And import error is triggered for importing from the submodule not the matgl lib itself:

        if calculator.lower() == "m3gnet":
            try:
                import matgl
>               from matgl.ext.ase import M3GNetCalculator

[DanielYang59]

I have to keep the test file name for borg as vasprun.xml.xe.gz instead of the expected vasprun.xe.xml.gz because it is hardcoded to collect info from vasprun files of pattern vasprun.xml*:

Well I came up with a solution in bd87dea: create a symlink with ScratchDir, now all files follow the same pattern 😃 .

[janosh]

perfect! thank you so much @DanielYang59! 👍 this is a huge PR and hugely appreciated! you've been a big help cleaning up the pymatgen code base!

i can take a look at the unrelated CI errors in a separate PR

[DanielYang59]

Thanks for reviewing @janosh! (I was about to redefine a VASP_OUT_DIR after cleaning up all vasp files, but glad you helped me out, thanks a ton 😃). I would proceed to other clean up once I have time and glad I could help.