materialsproject / pymatgen

Python Materials Genomics (pymatgen) is a robust materials analysis code that defines classes for structures and molecules with support for many electronic structure codes. It powers the Materials Project.
https://pymatgen.org
Other
1.48k stars 850 forks source link

When reading in a POTCAR file with pymatgen.io.vasp.inputs.Potcar, the object can't distinguish between PBE_54 and PBE. #3951

Open nhew1994 opened 1 month ago

nhew1994 commented 1 month ago

Python version

3.12.4

Pymatgen version

2024.6.10

Operating system version

Ubuntu-22.04

Current behavior

I have a POTCAR file written with PBE_54. When I read it into a Potcar object, potcar.functional will equal PBE instead of PBE_54.

Expected Behavior

I would expect it to be PBE_54 to reflect the actual POTCAR file being read.

Minimal example

from pymatgen.io.vasp.inputs import Potcar
path = "path_to_POTCAR_file"
potcar = Potcar.from_file(path)
print(potcar.functional)

Relevant files to reproduce this bug

Try any PBE_54 POTCAR file.

DanielYang59 commented 1 month ago

I think this is intended: https://github.com/materialsproject/pymatgen/blob/5256fce19f7366d33fd5b062bd41cc5cb713505f/src/pymatgen/io/vasp/inputs.py#L1776-L1788

Probably because POTCAR itself doesn't carry a one-to-one mapping to VASP version (but compatible functionals) itself with its LEXCH tag. I could be wrong here, so let's see if @esoteric-ephemera has any corrections.

I think the reason versioned functional tags exist in pymatgen is to help locate the corresponding POTCAR directories from local file system: https://github.com/materialsproject/pymatgen/blob/5256fce19f7366d33fd5b062bd41cc5cb713505f/src/pymatgen/io/vasp/inputs.py#L1757-L1763

nhew1994 commented 1 month ago

Hi @DanielYang59. Thank you for the reply.

At the moment, I am storing my results on my personal MongoDB database. When I read in a POTCAR file that I used with PBE_54, I would just like it to say PBE_54 to reflect the actual 54 version that I use instead of the old PBE.

I am wondering if there is any way around this. If not, that is ok.

DanielYang59 commented 1 month ago

No problem at all.

I wish I could be more helpful on this, but I don't have much experience with the functionality you were looking for. Hopefully others would have smarter solutions.

The functional property of PotcarSingle is not writable, however that of Potcar (a collection of PotcarSingle) is writable, therefore you could overwrite it directly:

from pymatgen.io.vasp.inputs import Potcar

potcar = Potcar.from_file("POTCAR")
potcar.functional = "PBE_54"

print(potcar.functional)  # >>> "PBE_54"

But you have to double check if such modification breaks any downstream operations (i.e. OPs that require functional to be exactly PBE).

If you just want to "tag" the Potcar, perhaps you could add a separate attribute:

potcar = Potcar.from_file("POTCAR")
potcar.functional_versioned = "PBE_54"

print(potcar.functional_versioned) # >>> "PBE_54"

Hope this helps.

nhew1994 commented 1 month ago

@DanielYang59 Thanks again for your help. I've opted for the first option (overwrite directly) for now.

Cheers.

DanielYang59 commented 1 month ago

All good, and do let me know if you (or any other) come up with smarter options :)

esoteric-ephemera commented 1 month ago

This has also confused me in the past but I don't know that it's a bug. The PotcarSingle class assumes instantiation from a string (or file), but has an optional from_symbol_and_functional instantiation method

Conversely, the Potcar class (a list of PotcarSingle's) assumes instantiation from symbols and a functional, with an optional from_file instantiation

It would probably make sense to have these be consistent in terms of what the default construction method is, but there's not enough reason to change this now and incur obvious breaking changes

So as @DanielYang59 said, the functional attr of any PotcarSingle reflects the DFT functional used to generate it (PBE or LDA)