Unhardcode LammpsDumpParser

[lilyminium]

Fixes below:

Amir can't load their LammpsDump file because atom lines have more than 5 fields.

>>> cell = mda.Universe(path, format='LAMMPSDUMP', atom_style='id type x y z xs ys zs')


ValueError                                Traceback (most recent call last)
/anaconda3/lib/python3.7/site-packages/MDAnalysis/core/universe.py in __init__(self, *args, **kwargs)
    289                     with parser(self.filename) as p:
--> 290                         self._topology = p.parse(**kwargs)
    291                 except (IOError, OSError) as err:
/anaconda3/lib/python3.7/site-packages/MDAnalysis/topology/LAMMPSParser.py in parse(self, **kwargs)
    609             for i in range(natoms):
--> 610                 idx, atype, _, _, _ = fin.readline().split()
    611 
ValueError: too many values to unpack (expected 5)

Changes made in this Pull Request:

-                  idx, atype, _, _, _ = fin.readline().split()
+                  idx, atype, *_ = fin.readline().split()

I know nothing about Lammps formats, so maybe atom lines having >5 fields is a sign that something is very wrong -- but then an informative error could be raised. Or possibly it should have at least 5 fields (for x, y, z)?

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

[codecov]

Codecov Report

Merging #2333 into develop will decrease coverage by 0.75%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           develop   #2333      +/-   ##
==========================================
- Coverage    90.56%   89.8%   -0.76%     
==========================================
  Files          170     173       +3     
  Lines        23100   21783    -1317     
  Branches      2985    2861     -124     
==========================================
- Hits         20921   19563    -1358     
- Misses        1581    1623      +42     
+ Partials       598     597       -1

Impacted Files	Coverage Δ
package/MDAnalysis/topology/LAMMPSParser.py	95.33% <100%> (-0.04%)	⬇️
package/MDAnalysis/analysis/lineardensity.py	62.38% <0%> (-20.74%)	⬇️
package/MDAnalysis/analysis/waterdynamics.py	82.46% <0%> (-8.18%)	⬇️
datafiles.py	26.66% <0%> (-4.59%)	⬇️
package/MDAnalysis/analysis/base.py	93.57% <0%> (-3.67%)	⬇️
...is/analysis/encore/clustering/ClusterCollection.py	62.9% <0%> (-3.23%)	⬇️
package/MDAnalysis/analysis/diffusionmap.py	90.62% <0%> (-2.71%)	⬇️
package/MDAnalysis/analysis/align.py	84.81% <0%> (-2.52%)	⬇️
package/MDAnalysis/analysis/contacts.py	97.56% <0%> (-2.44%)	⬇️
package/MDAnalysis/analysis/rms.py	90.66% <0%> (-2.14%)	⬇️
... and 104 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6fa3b6d...d6e904d. Read the comment docs.

[rkingsbury]

I have just run into this issue and personally would like to see this fix (or a sensible error message) implemented. It took me quite some time to understand why MDAnalysis was not able to load my ascii dump file

[rkingsbury]

MDAnalysis expects LAMMPSDUMP to be in the default 'atom' style https://userguide.mdanalysis.org/stable/formats/reference/lammpsdump.html#lammpsdump-format, which would have only 5 fields (atom ID, atom type, x, y, z) but it is not uncommon for users to output additional fields

[IAlibay]

@lilyminium I'm not super familiar with the format, do you happen to remember why this ended up stalling? (i.e. was there a practical reason or a lack of hands on deck to review?)

[IAlibay]

MDAnalysis expects LAMMPSDUMP to be in the default 'atom' style https://userguide.mdanalysis.org/stable/formats/reference/lammpsdump.html#lammpsdump-format, which would have only 5 fields (atom ID, atom type, x, y, z) but it is not uncommon for users to output additional fields

@rkingsbury I think part of the problem here is that most of our devs aren't familiar with the LAMMPS format. Do you happen to have a reference to some docs for the non-'atom' style formatting?

[rkingsbury]

@lilyminium I'm not super familiar with the format, do you happen to remember why this ended up stalling? (i.e. was there a practical reason or a lack of hands on deck to review?)

No, I have no idea. I've only recently started using MDAnalysis

[IAlibay]

@lilyminium I'm not super familiar with the format, do you happen to remember why this ended up stalling? (i.e. was there a practical reason or a lack of hands on deck to review?)

No, I have no idea. I've only recently started using MDAnalysis

Sorry for the confusion @rkingsbury that first question was for @lilyminium the dev who opened this pull request.

[lilyminium]

Python 3 concerns iirc (it didn't pass tests). This is one of the first PRs I ever made, back in the dark 2.7 days.

I also didn't push too hard because I'm not terribly familiar with LAMMPS. @rkingsbury is this a good solution or should anything be changed?

[hmacdope]

@lilyminium I have been working on this in parallel for #3358. Problem with current implementation in this PR being that there is no guarantee coordinates will be in the fields[-3:] spot, or that they will be in any given convention. There may also be multiple coordinate formats present. See the following for more info. https://docs.lammps.org/dump.html.

How would you like to proceed? I can work over the top of this PR if that is easiest or wait for this to be merged then put my own changes over the top. :)

[lilyminium]

I'm not too attached either way. Feel free to merge this branch into your own if that's easiest.

[amirhs1]

I contacted Lammps developers and they told me that
"Please also note that fix commands like fix property/atom allow to set custom section names for storing the associated properties. So technically speaking the list of section names is infinite."

However, they suggested me to see the documentations for these two commands (In particular, the 2nd one): read_dump and read_data

[hmacdope]

Note that majority of discussion on this has shifted to #3360

[hmacdope]

@lilyminium, with #3360 being merged I think this may be able to be closed.

[hmacdope]

@lilyminium closing