Bug - in lammps parser caused from using a dict

[JoshuaSBrown]

So I found a bug with the lammps parser it is a result of using a dictionary to store the parser arguments and then running it in the for loop. I will submit a patch but here is the initial documentation in case of future problems:

The error has been localized to the boundary and dimension parsers:

       parsable_keywords = {
            'units': self.parse_units,
            'atom_style': self.parse_atom_style,
            'dimension': self.parse_dimension,
            'boundary': self.parse_boundary,
            'pair_style': self.parse_pair_style,
            'kspace_style': self.parse_kspace_style,
            'pair_modify': self.parse_pair_modify,
            'bond_style': self.parse_bond_style,
            'angle_style': self.parse_angle_style,
            'dihedral_style': self.parse_dihedral_style,
            'improper_style': self.parse_improper_style,
            'special_bonds': self.parse_special_bonds,
            'read_data': self.parse_read_data}

Here they appear in the correct order. However, the dictionary does not imply that when you cycle through them they will appear in the same order as you have instantiated the key value pairs. Seeing that the boundary parser depends on the dimensions already being initiated this lead to a crash if the dict decides it wants to rearrange the order, which in my case it has done.

   def parse_boundary(self, line):
        """ """
        self.boundaries = [line[1], line[2], line[3]]
        if len(self.boundaries) != self.dimension:
            raise LammpsError("Boundaries do not match specified dimension "
                              "in input file")

[ctk3b]

Ah good catch - I believe I've run into this issue too. Is it fixed by simply using OrderedDict?

[JoshuaSBrown]

Well after taking a second look, the order of the dict does not appear the problem because it it reading the keywords from the lammps.input file in the order they appear and then plugging the keywords into the dict. This is a problem in this case if the boundary keyword appears before the dict keyword. Not quite sure yet what the best approach is to fixing this yet... any other suggestions.

[JoshuaSBrown]

Perhaps move all the error checking to after everything has been scanned/read in?

[ctk3b]

Perhaps move all the error checking to after everything has been scanned/read in?

That sounds like the correct solution.

[swails]

As a little tidbit - since Python 3.6, dict preserves insertion order.

No longer seems relevant here, but it may not be widely known.

[ctk3b]

And as of 3.7 it's part of the spec! 🎉