Issue 363 - New Topology System #815
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also, some other subtle bugfixes.
Somehow missed this one during the migration of old AtomGroup functions.
**NOTE:** `atoms.bonds` is broken for atom groups without explicit bond typing in this commit. In order to write a LAMMPS.DATAWriter, I first need explicit typing of bonds, angles, dihedrals, and impropers beyond the tupling of atoms types done now. To hack bond types into the topology, I branched issue-363 and tried the following: 1. Add optional `types` keyword to `topologyattrs.Bonds`. This is stored in the class instance a `self._bondtypes`. A topology parser can pass `types` to the Bonds topologyattr. 2. When `topologyattrs.Bonds` is called upon to generate a TopologyGroup, these values are passed on via a new keyword in `TopologyGroup.__init__()`. They are stored in the TopologyGroup instance as `self._bondtypes`. 3. I modified `TopologyGroup.__getitem__` to yield a Bond with attribute `_bondtype`. If `_bondtype is not None` then it overrides the default behavior of `bond.type`. 4. I modified `TopologyGroup()` calls wherever I could find them to pass on `_bondtype` in the instantiation. I found that the DATAParser in branch #363 is slightly broken, because the atom types are saved as integers which breaks `select_atoms` (I think it expects strings, and typing the types as strings on parsing fixes this issue). It seems there is currently no test for atom selection from LAMMPS DATA files, so this might have gone undetected. An error is raised for `ag.bonds` for an atom group that doesn't have bonds, but this seems to also be the case for branch issue-363 right now, so I didn't try to fix this. Two places are especially ugly. One is where I tried to hack `_bondtypes` into the existing code to remove duplicate bonds. This is also where the code seems to be broken for non-explicit bond types. In this case, I set `_bondtypes` to be a numpy array full of `None` with `dtype=object`. It seems that `unique_rows` fails for this case, although it works fine when `_bondtypes.dtype == "|S1"`.
The function `unique_row` was causing problems because bond type could be None, and the containing numpy array had dtype==object. I replaced `unique_rows` with `np.unique` by first converting the bond indices and bond types into a numpy record, and then breaking this record back into the bond indices and bond types. I also simplified some of what I'd written in `topologyattrs.Bonds`.
If parser told universe to create empty TopologyGroup, the resulting group was an attribute of atoms but gave IndexError on trying to access it. Seems to work as expected now... creating TopologyGroup with 0 TopologyObjects
Fix DATA type parsing and add explicit bond types
Writes selection at current trajectory frame to file, including sections Atoms, Masses, Velocities, Bonds, Angles, Dihedrals, and Impropers (if these are defined). Atoms section is written in the "full" sub-style if charges are available or "molecular" sub-style if they are not. Molecule id in atoms section is set to to 0. No other sections are written to the DATA file. As of this writing, other sections are not parsed into the topology by the `DATAReader`. If the selection includes a partial fragment, then the outputted DATA file will be invalid, because it will describe bonds between atoms which do not exist in the Atoms section. By default the writer assumes "conventional" or "real" LAMMPS units where length is measured in Angstroms and velocity is measured in Angstroms per femtosecond. If other units are desired, they must be specified. Raises ValueError if atom types are not convertable to integers or if atoms of the same type don't all have the same mass. An AttributeError will be raised if the atom group doesn't have masses. I added tests to make sure topology attributes of the written file (after being read again) match the topology attributes read from the original file. Checks types, bonds, positions, velocities, etc. There was a problem with test data mini.data because it had atom type 3 which had undefined mass, so I changed the atom type to 1. The DATAParser before was not converting velocity units, so I added default units Angstroms/fs which are the "real" units in LAMMPS.
I added an associated test and fixed part of a test that was passing in an invalid way.
I also removed the part of the docstring describing the not-yet-implemented feature because it would probably cause more false hope than good.
This in accordance with the consensus on #599 (#599 (comment)).
We want ResidueGroups and SegmentGroups to behave roughly in the same way as before, that is having methods they had when they were subclasses of AtomGroup. Not everything makes sense, but most things do. This was consensus from discussion in #703 (#703 (comment))
I consider these very fragile since they depend on particular atom names (however customary), but included them where they should go since they depend on atom names. They will need to be refactored, though, since they depend on getitem working for atom names, which is not something we've included (so far) for any groups.
Probably useful for PDB files or any other topology file that gives an element name.
Conflicts: package/MDAnalysis/coordinates/MOL2.py package/MDAnalysis/coordinates/PDB.py package/MDAnalysis/core/AtomGroup.py package/MDAnalysis/core/Selection.py package/MDAnalysis/core/Timeseries.py package/MDAnalysis/core/__init__.py package/MDAnalysis/core/topologyobjects.py package/MDAnalysis/lib/util.py package/MDAnalysis/topology/CRDParser.py package/MDAnalysis/topology/DLPolyParser.py package/MDAnalysis/topology/GMSParser.py package/MDAnalysis/topology/GROParser.py package/MDAnalysis/topology/HoomdXMLParser.py package/MDAnalysis/topology/PDBParser.py package/MDAnalysis/topology/PDBQTParser.py package/MDAnalysis/topology/PSFParser.py package/MDAnalysis/topology/PrimitivePDBParser.py package/MDAnalysis/topology/TOPParser.py package/MDAnalysis/topology/XYZParser.py package/MDAnalysis/topology/base.py package/MDAnalysis/topology/core.py package/MDAnalysis/topology/tpr/utils.py testsuite/MDAnalysisTests/test_atomselections.py testsuite/MDAnalysisTests/test_topology.py testsuite/MDAnalysisTests/test_util.py testsuite/MDAnalysisTests/topology/test_gro.py testsuite/MDAnalysisTests/topology/test_tprparser.py
This is to match decided behavior for accessing attributes at various levels.
|
OMG, this is really happening? |
Readded tests for phi_selection and friends
* fix bug and docs in rotaxis If a == b this used to return [nan, nan, nan] due to the division by 0. * pep8 changes test_transformations.py * add rotaxis test * add align_principal_axis test * update changelog
Good idea, because it's been a common idiom to use AtomGroup.{AtomGroup,Universe} Oliver Beckstein * orbeckst@gmx.net |
abiedermann
pushed a commit
to abiedermann/mdanalysis
that referenced
this pull request
Oct 26, 2016
NIMUstA8lOQd9w5kL+PcYC3o+J7vh1/gjIyJsZ5iE9y9zQ3aYtzFQShPnCfrKYyl Bco/xr+U3bN9piXTZqhYKIRzVUs6SSd2uy7q63de8QDNsNWkKouKZ1+PhvZPkMNN i+PhBW/gp+PXeta+4Y5REnBrUpX4bW3DCHuKTJ+nM80PXmsMG0i64ShRX/umXnoG qpBfOGfnr40SD/ZFmmYc8qqZvllzPjew8GMGXXdjidetOZHaAkFRDMzOr3FNYkWD 2zSKN95CJGDynSwdsDTo9rrUN5lcJr58JG+JeDBLoK7XxN5VVhPge+cV0gHF2SLk TQcgRXXY/AJY0ZpME0PRk7YKpUPqW/UjKPHENRFqNPlskF+8dzvFu2KNk928A+Ws 4otsRWCzn76tzXsGAK66kXsX9l2mc3IxhDy9TH69GXhog7ASHlglJZ1kZTcPPR8T h7SfjW1Pfi7C/HL7RJ+shmmIQd4qvfn828JK/SoHud6dG7/wmfqYIiva7RB0usdt CK8y7an2XUGXjnvIv2C2CDUHKy0KgtdRw6wpwChkeVl9ci59cT0vHkOZqXgGSaBS L168OTkm0djbpjrMGj8vWe3lna1/Fxf+ELYOC/rUTPOYt7T0SuRoUHhcHhURCD/w vViD4Ic6nvs7OR+ODkyZ =eUcw -----END PGP SIGNATURE----- adjusted HOLE open file descriptor test (MDAnalysis#129) for Darwin In order to provoke the failure described by issue MDAnalysis#129 we artificially lower the open file descriptors. On Mac OS X on travis this always leads to failing tests, as discussed under MDAnalysis#901 (comment) ; this hack increases the allowed open fds when we are running on Darwin. According to @jbarnoud, this problem is solved by MDAnalysis#363 so once that corresponding PR MDAnalysis#815 is merged we should be able to revert this commit.
4 tasks
* Add tests * TST: Added tests for get_named_residue * fix imports and pylint warning
* Finished checking PQRWriter * MOL2Writer now cleanly doesn't write non mol2 sources * Finished checking PDBQT
Conflicts: package/AUTHORS package/CHANGELOG testsuite/MDAnalysisTests/analysis/test_psa.py
|
@kain88-de sorry I missed yesterday, you can merge this when ready |
|
OK the new topology system is now in develop. Everyone has done a great job. We even managed to increase coverage with this massive PR. |
|
Wow! Congrats! On 07-11-16 12:09, Max Linke wrote:
|
orbeckst
changed the title
|
Epic merge! Special kudos to @richardjgowers, @dotsdl and @kain88-de. It's been almost 1 year since @richardjgowers visited ASU and started the topology overhaul together with @dotsdl. They got the basics done in about 1 month
but it took another ~45 weeks to make it the new foundation for MDAnalysis. The 10/90 rule at work...
|
|
Incredible! |
|
Wooo! This is a big deal! Thanks everyone for the hard work on all this. It really was a great community effort, and really reflects well on the mettle of our dev team. I mean, look at that participant list on the right. Looking forward to what we can make happen with the new topology system. This enables tons of new possibilities. :D |
abiedermann
pushed a commit
to abiedermann/mdanalysis
that referenced
this pull request
Jan 5, 2017
WIP: Issue 363 - New Topology System
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #363
This is a WIP PR to serve as a focal point for discussion on the new topology system before it is merged into
develop. The merge will happen soon after release of 0.15.0.Changes made in this Pull Request:
TODO List