Skip to content

Fix structure field shadowing sort_structure in VaspInputGenerator - #1530

Open
ThomasWarford wants to merge 3 commits into
materialsproject:mainfrom
ThomasWarford:fix/sort-structure-shadow
Open

Fix structure field shadowing sort_structure in VaspInputGenerator#1530
ThomasWarford wants to merge 3 commits into
materialsproject:mainfrom
ThomasWarford:fix/sort-structure-shadow

Conversation

@ThomasWarford

@ThomasWarford ThomasWarford commented Aug 21, 2026

Copy link
Copy Markdown

Summary

  • VaspInputGenerator redeclared structure as a plain dataclass field, overriding the structure property (with its sort_structure/reduce_structure/validate_magmom setter) inherited from pymatgen's VaspInputSet. As a result, sort_structure silently never ran for any atomate2 VASP input-set generator.
  • This surfaces destructively in MDSetGenerator's npt ensemble: LANGEVIN_GAMMA is sized to the number of distinct elements, while VASP counts POSCAR "types" as contiguous same-element runs. With an unsorted structure the two counts diverge and VASP aborts with Error reading item LANGEVIN_GAMMA from file INCAR.
  • Fix: remove the redundant field declaration so the inherited property is no longer shadowed.
  • Added regression tests (tests/vasp/test_sets.py) and a standalone repro (example.py).

Alternatively, we modify MDSetGenerator something like:

class MDSetGenerator(VaspInputGenerator):
    def get_input_set(self, structure=None, prev_dir=None, potcar_spec=False):
        if structure is not None and self.sort_structure:
            structure = structure.get_sorted_structure()
        return super().get_input_set(structure, prev_dir=prev_dir, potcar_spec=potcar_spec)

for a more targeted fix.

Additional dependencies introduced (if any)

  • None.

TODO (if any)

  • None.

Checklist

Work-in-progress pull requests are encouraged, but please put [WIP] in the pull request
title.

Before a pull request can be merged, the following items must be checked:

  • Code is in the standard Python style.
    The easiest way to handle this is to run the following in the correct sequence on
    your local machine. Start with running ruff and ruff format on your new code. This will
    automatically reformat your code to PEP8 conventions and fix many linting issues.
  • Doc strings have been added in the Numpy docstring format.
    Run ruff on your code.
  • Type annotations are highly encouraged. Run mypy to
    type check your code.
  • Tests have been added for any new functionality or bug fixes.
  • All linting and tests pass.

Note that the CI system will run all the above checks. But it will be much more
efficient if you already fix most errors prior to submitting the PR. It is highly
recommended that you use the pre-commit hook provided in the repository. Simply run
pre-commit install and a check will be run prior to allowing commits.

Issue

Closes #1529. See this issue for bug reproduction instructions.

@JaGeo

JaGeo commented Aug 22, 2026

Copy link
Copy Markdown
Member

@esoteric-ephemera do you have time to look into this? I don't have much experience with the MD workflow

Comment on lines +176 to +177
# Do not redeclare `structure` here: VaspInputSet so setting structure
# uses pymatgen.io.vasp.sets setter which sorts the structure

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the ai-generated comment please

@esoteric-ephemera

Copy link
Copy Markdown
Collaborator

Thanks @ThomasWarford. I'm not sure the analysis is quite right, since VaspInputSet itself defines structure as an init field, and then later defines it as a property with a setter. My guess is that there's some unhealthy handling/collision of fields in the upstream model that need to be changed

That being said, it is redundant to redeclare the field here so I'm fine with this change

Guessing example.py was an AI-generated MWE?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: VaspInputGenerator.structure field shadows sort_structure property, breaking MD (npt) LANGEVIN_GAMMA

3 participants