Skip to content

[Code scan] Make AbacusLite property keyword conflict checks effective #7546

Description

@njzjz

This issue is a result of a Codex global repository scan.

AbacusTemplate.get_property_keywords() appears intended to detect contradictory keyword requirements between requested ASE properties, but the param_cache_ dictionary is never updated. As a result, counter() can only compare against an empty cache and will not detect conflicts between property keyword sets.

def get_property_keywords(self,
parameters: Dict[str, str],
properties: List[str]) -> Dict[str, str]:
'''Connect the relationship between the properties calculation and
the ABACUS keywords. May be more complicated in the future, therefore
it is better to have a seperate mapping function instead of
implementing in some other functions.
Parameters
----------
parameters : dict
The parameters used to perform the calculation.
properties : list of str
The list of properties to calculate
'''
# update the parameters with the keywords for the properties
# however, one should also consider that there may be the case that
# contradictory keywords are needed. In this kind of cases,
# we should raise a ValueError
param_cache_ = {}
def counter(param_new: Dict[str, str]) -> Dict[str, str]:
info = 'desired properties required contradictory keywords'
for k, v in param_new.items():
if k in param_cache_ and param_cache_[k] != v:
raise ValueError(f'{info}: {k}={v} (now), {param_cache_[k]} (before)')
# if it is alright, pass through
return param_new
# update the parameters with the keywords for the properties
for p in properties:
assert p in self.implemented_properties
parameters.update(counter(getattr(self, f'get_{p}_keywords')(parameters)))

Relevant code:

param_cache_ = {}
def counter(param_new: Dict[str, str]) -> Dict[str, str]:
    info = 'desired properties required contradictory keywords'
    for k, v in param_new.items():
        if k in param_cache_ and param_cache_[k] != v:
            raise ValueError(f'{info}: {k}={v} (now), {param_cache_[k]} (before)')
    return param_new

for p in properties:
    assert p in self.implemented_properties
    parameters.update(counter(getattr(self, f'get_{p}_keywords')(parameters)))

Impact:

If two property handlers require different values for the same ABACUS keyword, the later parameters.update(...) silently wins instead of raising the documented ValueError. This can produce an input file that does not match one of the requested properties.

Suggested fix:

Update param_cache_ after validating each property keyword set, and consider checking against user-provided parameters as well if a requested property would overwrite an explicit user keyword.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions