Read INCAR SYSTEM as is, check_params use proc_val

[DanielYang59]

Summary

• Manually update incar_parameters.json, to fix VaspInput setter and Incar.check_params() are inconsistent #4119 (comment), need a more systematic script to update incar_parameters.json in the long run
• check_params use proc_val
• Remove some unnecessary casting in INCAR values as we're confident they are capitalized now
• Avoid casing INCAR SYSTEM keyword, to fix Make Incar keys case insensitive, fix init Incar from dict val processing for str/float/int #4122 (comment)

[DanielYang59]

I believe this would be covered by

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

Line 1029 in 6992aee

return val.strip().capitalize()

[DanielYang59]

After #4122 we should be confident that string values are now capitalized (except for those in lower_str_keys and as_is_str_keys), these case conversion may not be needed.

[yantar92]

If they are capitalized, why comparing with "Bse" (non-capitalized)

[DanielYang59]

Sorry I may not understand your question? The value Bse is capitalized? The key ALGO is in uppercase.

print("BSE".capitalize())  # Bse

[yantar92]

Sory, I got completely confused. Mostly because of apparent inconsistency between the code and VASP wiki. If you look at https://www.vasp.at/wiki/index.php/ALGO, you will see that BSE and many other values are all capitals:

ALGO = Normal | VeryFast | Fast | Conjugate | All | Damped | Subrot | Eigenval | Exact | None | Nothing | CHI | G0W0 | GW0 | GW | scGW0 | scGW | G0W0R | GW0R | GWR | scGW0R | scGWR | ACFDT | RPA | ACFDTR | RPAR | BSE | TDHF

So, the code reads awkward now.

[DanielYang59]

I got completely confused. Mostly because of apparent inconsistency between the code and VASP wiki

Yes I find it confusing too #4122 (comment), and @esoteric-ephemera said it's a convention to pymatgen #4122 (comment)

This is likely like this since the start:

As VASP itself is largely case insensitive, we just need to internally standardize for easy handling (no need to cast to upper/capitalize when comparing values and so on). I personally prefer to cast both value and tag to uppercase for the following reason:

• VASP Wiki large prefer upper case for keys, and get rid of the headache to memorize "key is uppercase", but "case is capitalized" and so on
• Most values (just like keys) are first-letter abbreviations of some sort, capitalizing them doesn't make much sense, though there're values that is indeed proper words like "Fast/Conjugate/All/..."

But this would be badly breaking (every downstream package would find the INCAR key in a different case) so I guess it's not worth the price. So we'd better come up with something that is not breaking. In any case I would leave this for a future PR.

---

I'm not sure it's doable or not, a good alternative is to subclass a "case insensitive string" as an inplace replacement of built in str to avoid the headache altogether (but I currently don't have a good way to overwrite the membership check), I might have another try later.

class CaseInsensitiveStr(str):
    def __new__(cls, value):
        return super().__new__(cls, value.casefold())

    def __eq__(self, other):
        if isinstance(other, str):
            return super().__eq__(other.casefold())
        return NotImplemented

    def __hash__(self):
        return hash(self.casefold())

str_a = CaseInsensitiveStr("HELLO")

print(str_a)  # hello

print(str_a == "hello")  # True
print(str_a == "HELLO")  # True

print(str_a in {"hello", "world"})  # True
print(str_a in {"HELLO", "world"})  # False, should be True
print(str_a in {"world"})  # False

[yantar92]

Yes I find it confusing too #4122 (comment), and @esoteric-ephemera said it's a convention to pymatgen #4122 (comment)

I see, and I agree that abstracting things out into case-sensitive/insensitive class is the most robust way to go.

As for the convention, it annoyingly has exceptions:

class Incar(UserDict, MSONable):
    """
    A case-insensitive dictionary to read/write INCAR files with additional helper functions.

    - Keys are stored in uppercase to allow case-insensitive access (set, get, del, update, setdefault).
    - String values are capitalized by default, except for keys specified
        in the `lower_str_keys` of the `proc_val` method.
    """

As you can see, capitalization is documented, except "keys specified in lower_str_keys". Currently it is a single key - ML_MODE.

However, do note that Incar.check_params does not expect such exceptions:

            if allowed_values is not None:
                # Note: param_type could be a Union type, e.g. "str | bool"
                if "str" in param_type:
                    allowed_values = [item.capitalize() if isinstance(item, str) else item for item in allowed_values]

(aside: incar_parameters.json does not even list ML_MODE)

[DanielYang59]

As you can see, capitalization is documented, except "keys specified in lower_str_keys". Currently it is a single key - ML_MODE.

Yep I updated the docstring to reflect this key/value case issue, but I'm not sure about the casing of ML_MODE either (I thought VASP is largely case insensitive ) #4122 (comment)

I never use that tag so cannot comment, it would be great if you (or someone else) could confirm this behaviour.

However, do note that Incar.check_params does not expect such exceptions:

Thanks I would update it (one exception makes everything hard to handle)

[yantar92]

I never use that tag so cannot comment, it would be great if you (or someone else) could confirm this behaviour.

I do not use this tag either, but clearly its value is case-sensitive - #3625
I looked up https://www.vasp.at/wiki/index.php/ML_MODE and it says

Warning: If any option other than the above is chosen or there is a spelling error (be careful to write everything in upper case or lower case letters) the code will exit with an error.

I conclude that case-insensitivity is not universal.

Ironically, simply upcasing (rathen than capitalizing) everything would not require making an exception for ML_MODE.

[DanielYang59]

I do not use this tag either, but clearly its value is case-sensitive - #3625

Yes I did see this PR and the VASP comment as well.

I conclude that case-insensitivity is not universal.
Ironically, simply upcasing (rathen than capitalizing) everything would not require making an exception for ML_MODE.

I believe ML_MODE is the only exception for now. upcase everything (except for SYSTEM) would make everything easy, but I guess ML_MODE was not there when this part of code was written (12 years ago), and it's hard to change it now without breakage

[yantar92]

t's hard to change it now without breakage

100% agree.

[DanielYang59]

@yantar92 Hope this looks good to you

The issue with incar_parameters.json is not completely resolved as we cannot reply on manual upgrade of tags and values, a script is certainly needed but I don't have time to work on this at this moment.

[yantar92]

a script is certainly needed but I don't have time to work on this at this moment.

#4119 (comment)

[shyuep]

I am merging this. In the end, we are using the 80/20 rule. VASP keys are always all caps. VASP values are a mess of different things (int, floats, case-insensitive strings, case-sensitive strings, etc.). For now, we will just use capitalize since that works for most of the cases, e.g., ALGO=Fast,Normal, etc. I get that BSE should be full caps since it is an acronym but since it makes no difference to actual functionality, we are going to live with it.