363 - Check Writers all still work

[richardjgowers]

This is an umbrella issue for all the Writer checks for the 363 merge

All writers should still work, regardless of the AtomGroup/Universe supplied (as before). This might (now) necessitate some guessing of attributes, (PDBWriter requires resids, these might not exist).

For this, each Writer need checking! Each Writer has a card in the project board. When you start checking a writer, move the card to in progress to avoid duplication of efforts. If a Writer proves stubborn, you can convert the card into an issue and raise the problem.

Requirements:

• each Writer needs tests!
• each Writer should work when given a blank (e: ie without any Attributes) AtomGroup (see core.groupbase.make_Universe to generate Universes with arbitrary attributes)!

[kain88-de]

XTC and TRR are still OK

[tylerjereddy]

I'm looking at GRO -- should universe.residues[-1].atoms.resids = resid_value work in 363?

I'm getting a traceback indicating that set_atoms is not implemented in topologyattrs.py.

[richardjgowers]

Yeah so resid can only be set via the Residue/ResidueGroup, e: so you'd have to do universe.residues[-1].resid = resid_value

[tylerjereddy]

Looks like the 363 branch doesn't currently support setting resid values of arbitrarily large size?

Traceback (most recent call last):
  File "/Users/treddy/python_modules/MDAnalysis/MDA_dev/mdanalysis/testsuite/MDAnalysisTests/coordinates/test_gro.py", line 300, in test_writer_large_residue_count
    self.large_universe.residues[-1].resid = resid_value
  File "/Users/treddy/python_modules/MDAnalysis/MDA_dev/mdanalysis/package/MDAnalysis/core/groups.py", line 141, in setter
    return attr.__setitem__(self, values)
  File "/Users/treddy/python_modules/MDAnalysis/MDA_dev/mdanalysis/package/MDAnalysis/core/topologyattrs.py", line 94, in __setitem__
    return self.set_residues(group, values)
  File "/Users/treddy/python_modules/MDAnalysis/MDA_dev/mdanalysis/package/MDAnalysis/core/topologyattrs.py", line 887, in set_residues
    self.values[rg._ix] = values
OverflowError: Python int too large to convert to C long

[richardjgowers]

Hmm, that looks like you're overflowing a np.int64, I think python will have previously just changed the single resid value to be a bigger int tye before. int64 goes up to 2 billion, so it shouldn't be a problem in real cases

[tylerjereddy]

My vote is for the core code to handle the data type promotion automatically. Probably first by trying to promote to np.int128, and perhaps failing that--fall back to using python ints (dtype=object), with whatever performance implications are incurred, as that will restore the previous behaviour for any strange cases like mine.

Although in practice the fallbacks won't likely ever occur, so performance won't be affected in any substantial way, it does enable testing flexibility and general robustness.

Looks like it is this block of code in MDAnalysis/core/groups.py:

 193 class GroupBase(_MutableBase):                                                  
 194     """Base class from which a Universe's Group class is built.                 
 195                                                                                 
 196     """                                                                         
 197     def __init__(self, ix, u):                                                  
 198         # indices for the objects I hold                                        
 199         self._ix = np.asarray(ix, dtype=np.int64)

Conversely, if enough people object I can just remove some digits from the original unit test (which I wrote a few months ago).

[kain88-de]

I would say that int64 is more then big enough for the not to distant future.

>>> np.iinfo(np.int64)
iinfo(min=-9223372036854775808, max=9223372036854775807, dtype=int64)

I don't think we would be close to this number simulating cells with about a Quintilian different atoms. Also there is no int128 in numpy (Which would go up to ~1.7e38 that is roughly enough to number every atom in every human being, assuming each human has ~ 100 e26 atoms [if they weight around 100 Kg])

[kain88-de]

So I just checked your test number in the gro-file test. It is really to large for an int64. I think you can safely change the number to np.iiinfo(np.int64).max. If int64 is large enough to count every atom in a cell it should be more then enough for numbering residues.

[tylerjereddy]

Ok, fair enough. It is mostly designed to test truncation behaviour, not any real number of particles.

[tylerjereddy]

@richardjgowers What behaviour do you want for empty_ag.write('test.gro')? Currently, this produces: IndexError: Cannot write an AtomGroup with 0 atoms. Are you saying you want a .gro file with no atoms in it to be produced?

[richardjgowers]

@tylerjereddy sorry, what I meant by blank was without any topology attributes, so for example with GRO I didn't assume there'd be resnames and caught and warned about the output.