Make Incar keys case insensitive, fix init Incar from dict val processing for str/float/int

[DanielYang59]

Summary

• Fix Incar.check_params handling of case sensitivity, to fix VaspInput setter and Incar.check_params() are inconsistent #4119, also add unit tests
• Issue warning if duplicated keys found when init Incar

Make more operations (get/update/setdefault) in Incar case insensitive

• Init Incar from dict (also the from_dict method) doesn't seem to use setter method, might be better off subclass UserDict
• Update Incar class docstring to record the change in behaviour
• Test other dict methods like update/setdefault/...

Init Incar from dict doesn't call setter method, leading to unprocessed str/float/int value

• Setter method only trigger proc_val when val is string, leading to inconsistent behaviour when init from dict for int/float
• Setter method not triggered on update method when subclassing builtin dict, leading to val not passing through proc_val when init from dict

[DanielYang59]

ping the true expert @esoteric-ephemera in case you have any comments (perhaps deserve a separate PR):

• Do we want to cast INCAR keys to upper case (to be consistent with VASP behaviour, but potentially breaking)
• Do we want to automatically filter duplicated INCAR keys (with a warning of course), again to be consistent with VASP behaviour (this should be resolved by casting keys to upper, as we cannot have duplicated keys in dict)
• Wondering why do we capitalize values for unknown string keys?

  pymatgen/src/pymatgen/io/vasp/inputs.py

  Line 988 in 32d9beb

  return val.strip().capitalize()

Thanks in advance :)

[esoteric-ephemera]

Do we want to cast INCAR keys to upper case (to be consistent with VASP behaviour, but potentially breaking)

Why do this exactly? FORTRAN (unless you use specific compiler options) is case insensitive, and most FORTRAN code, VASP included, is case insensitive. aLgO = aLL should mean the same thing as ALGO = ALL to VASP

Do we want to automatically filter duplicated INCAR keys (with a warning of course), again to be consistent with VASP behaviour (this should be resolved by casting keys to upper, as we cannot have duplicated keys in dict)

I would just throw a user warning. Because of how VASP parses an INCAR, I'm pretty sure you can write the same tag multiple times, and the bottom-most value is the one that's accepted

Wondering why do we capitalize values for unknown string keys?

Probably just convention, again because FORTRAN is case insensitive

Also for the check_incar stuff: keep in mind that FORTRAN is very strictly typed. Even if I write ENCUT = 500 in the INCAR, fortran will enforce this to be double precision because ENCUT is declared as a double

[JaGeo]

I btw recently had issues with updating the incar. I would have expected that sigma and SIGMA would simply modify SIGMA but they did not. Instead, i had two different sigmas in the incar. I did not have time to investigate but it was a very recent pymatgen version

[esoteric-ephemera]

@JaGeo did VASP parse them as different variables or just take the one defined furthest down the INCAR?

[shyuep]

Yes, all tags are case insensitive. I don't think the Incar class actually checks. So if you set sigma and SIGMA, you end up with two sigma tags. The easiest solution is to automatically capitalize the key in setitem for Incar. I would use Camelcase for the values for Incar (even though it makes no difference where VASP is concerned). Usually VASP just looks at the first letter. E.g., ALGO = Accurate and ALGO=A means the same thing.

[DanielYang59]

Probably just convention, again because FORTRAN is case insensitive

Thanks that makes sense, and I don't think we need to change this as it wouldn't have any side effect.

I'm pretty sure you can write the same tag multiple times, and the bottom-most value is the one that's accepted

did VASP parse them as different variables or just take the one defined furthest down the INCAR?

I believe VASP parses the first occurrence (can you confirm it's the bottom one? @esoteric-ephemera ), but I could be wrong here. I tested with INCAR:

encut = 500
ENCUT = 400

And grep ENCUT vasprun.xml would return: <i name="ENCUT"> 500.00000000</i>

[JaGeo]

@DanielYang59 @esoteric-ephemera Yes, it took the first one as far as I remember.

@shyuep i might be wrong but I thought i modelled the lobsterin class after the incar at the time and i think it checked at some point. In any case, it would be great if this would be checked again (e.g., via Camelcase).
EDIT: ah, already implemented 😀

[DanielYang59]

Why do this exactly?

I don't think the Incar class actually checks. So if you set sigma and SIGMA, you end up with two sigma tags. The easiest solution is to automatically capitalize the key in setitem for Incar.

I proposed that we cast Incar keys to upper case internally in order to prevent such duplicate and make other internal checking/handling easier.

Also the reason I personally prefer upper case over others (capitalize/lower) being:

• Most (if not all) our internal recordings (incar_parameters.json, and all kinds of recordings inside vasp.inputs class) use upper case, so we don't need to update them all
• VASP internally cast all keys to upper case when parsing (see the top comment in VASP source code drdatab.F)
• VASP wiki also use upper case, so I assume it's more or less a convention?

[shyuep]

@JaGeo I just fix the upper case keys problem and added a test. It should no longer be a problem. As far as I can tell, the values are already properly capitalize and there is a test.

[DanielYang59]

This change:

- val = self.proc_val(key, str(val)) if isinstance(val, str) else val
+ val = self.proc_val(key, str(val)) if isinstance(val, str | float | int) else val

should have a healthy dose of stupidity as I believe casting int/float to str is necessary such that the val would go through proc_val to sanitize its type, otherwise it would bypass the proc_val check, leading to inconsistent behaviour when init from dict vs from str/file.

[DanielYang59]

Before migrating to subclassing UserDict, the setter method is not triggered through update method when init from a dict, therefore these values remain as is (BSE) when it should be capitalized (Bse).

pymatgen/src/pymatgen/io/vasp/inputs.py

Lines 718 to 737 in ed8542d

	def __init__(self, params: dict[str, Any] | None = None) -> None:
	"""
	Create an Incar object.
	Args:
	params (dict): Input parameters as a dictionary.
	"""
	super().__init__()
	if params is not None:
	# If INCAR contains vector-like MAGMOMS given as a list
	# of floats, convert to a list of lists
	if (params.get("MAGMOM") and isinstance(params["MAGMOM"][0], int | float)) and (
	params.get("LSORBIT") or params.get("LNONCOLLINEAR")
	):
	val = []
	for idx in range(len(params["MAGMOM"]) // 3):
	val.append(params["MAGMOM"][idx * 3 : (idx + 1) * 3])
	params["MAGMOM"] = val
	self.update(params)

[shyuep]

Thanks.

[DanielYang59]

No problem, thanks for the helpful discussion everyone

[yantar92]

I think that one special key is SYSTEM - it is usually free text and capitalizing it can be awkward.

[DanielYang59]

I think that one special key is SYSTEM - it is usually free text and capitalizing it can be awkward.

That's a good point to me.

Update: I released comparing two INCARs (especially to check if they produce the same result) through directly == may not make sense as the VASP default value handling, so it should be preferred to use diff for a manually inspection.

Meanwhile, I think the __eq__ for Incar/Kpoint (perhaps diff as well? I don't have a strong opinion on diff though) should be overwritten to ignore the comment/SYSTEM for similar reason (docstring should be updated to reflect this behaviour of course).