Add warning for matching atoms without masses (#2614)

* Fixes issue #2469
* raise SelectionWarning if AlignTraj is called with match_atoms=True and masses are not present
* fit_translation and fit_trans_rot now raise TypeErrors (originally ValueErrors) for getting weights from atoms without masses
* get_weights now raises ValueError if weights is given as incorrect value (originally TypeError)

Co-authored-by: Philip Loche <ploche@physik.fu-berlin.de>
Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>

package/AUTHORS

@@ -141,6 +141,7 @@ Chronological list of authors
   - Morgan L. Nance
   - Faraaz Shah
   - Wiep van der Toorn
+  - Siddharth Jain
 
 
 External code

package/CHANGELOG

@@ -22,6 +22,7 @@ mm/dd/yy richardjgowers, kain88-de, lilyminium, p-j-smith, bdice, joaomcteixeira
   * 0.21.0
 
 Fixes
+  * Proper error message for AlignTraj on trajectory without mass (Issue #2469)
   * Updated tests to have explicit fixtures (Issue #2618)
   * XDR offsets now read from trajectory if offsets file read-in fails on
     IOError (Issue #1893, PR #2611)
@@ -34,7 +35,7 @@ Fixes
   * Fixed mda.Merge for Universes without coordinates (Issue #2470)(PR #2580)
   * PCA(align=True) now correctly aligns the trajectory and computes the
     correct means and covariance matrix (Issue #2561)
-  * Correct args order of base.AnalysisFromFunction (Issue #2503)  
+  * Correct args order of base.AnalysisFromFunction (Issue #2503)
   * encore.dres() returns dimensionality reduction details instead of a
     reference to itself (Issue #2471)
   * Handle exception when PDBWriter is trying to remove an invalid StringIO

package/MDAnalysis/analysis/align.py

@@ -411,7 +411,7 @@ def alignto(mobile, reference, select=None, weights=None,
        :class:`~MDAnalysis.core.groups.AtomGroup` with defined atom order as
        described under :ref:`ordered-selections-label`).
     match_atoms : bool (optional)
-        Whether to match the mobile and reference atom-by-atom. Default ``True``. 
+        Whether to match the mobile and reference atom-by-atom. Default ``True``.
     weights : {"mass", ``None``} or array_like (optional)
        choose weights. With ``"mass"`` uses masses as weights; with ``None``
        weigh each atom equally. If a float array of the same length as
@@ -569,7 +569,7 @@ def __init__(self, mobile, reference, select='all', filename=None,
         tol_mass : float (optional)
             Tolerance given to `get_matching_atoms` to find appropriate atoms
         match_atoms : bool (optional)
-            Whether to match the mobile and reference atom-by-atom. Default ``True``. 
+            Whether to match the mobile and reference atom-by-atom. Default ``True``.
         strict : bool (optional)
             Force `get_matching_atoms` to fail if atoms can't be found using
             exact methods
@@ -701,7 +701,7 @@ def _conclude(self):
 
 
 class AverageStructure(AnalysisBase):
-    """RMS-align trajectory to a reference structure using a selection, 
+    """RMS-align trajectory to a reference structure using a selection,
     and calculate the average coordinates of the trajectory.
 
     Both the reference `reference` and the trajectory `mobile` must be
@@ -710,7 +710,7 @@ class AverageStructure(AnalysisBase):
     current frame.
 
     The output file format is determined by the file extension of
-    `filename`. 
+    `filename`.
 
     Example
     -------
@@ -753,7 +753,7 @@ def __init__(self, mobile, reference=None, select='all', filename=None,
         tol_mass : float (optional)
             Tolerance given to `get_matching_atoms` to find appropriate atoms
         match_atoms : bool (optional)
-            Whether to match the mobile and reference atom-by-atom. Default ``True``. 
+            Whether to match the mobile and reference atom-by-atom. Default ``True``.
         strict : bool (optional)
             Force `get_matching_atoms` to fail if atoms can't be found using
             exact methods
@@ -768,7 +768,7 @@ def __init__(self, mobile, reference=None, select='all', filename=None,
         ref_frame : int (optional)
             frame index to select frame from `reference`
         verbose : bool (optional)
-            Set logger to show more information and show detailed progress of 
+            Set logger to show more information and show detailed progress of
             the calculation if set to ``True``; the default is ``False``.
 
 
@@ -785,7 +785,7 @@ def __init__(self, mobile, reference=None, select='all', filename=None,
         rmsd : float
             Average RMSD per frame
         filename : str
-            String reflecting the filename of the file where the average 
+            String reflecting the filename of the file where the average
             structure is written
 
 
@@ -882,7 +882,7 @@ def _single_frame(self):
                              self.mobile,
                              mobile_com,
                              self._ref_com, self._weights)[1]
-        self.positions += self.mobile_atoms.positions            
+        self.positions += self.mobile_atoms.positions
 
     def _conclude(self):
         self.positions /= self.n_frames
@@ -1350,45 +1350,50 @@ def get_atoms_byres(g, match_mask=np.logical_not(mismatch_mask)):
 
             # stop if we created empty selections (by removing ALL residues...)
             if ag1.n_atoms == 0 or ag2.n_atoms == 0:
-                errmsg = ("Failed to automatically find matching atoms: created empty selections. " +
+                errmsg = ("Failed to automatically find matching atoms: created empty selections. "
                           "Try to improve your selections for mobile and reference.")
                 logger.error(errmsg)
                 raise SelectionError(errmsg)
-                
+
     if match_atoms:
         # check again because the residue matching heuristic is not very
         # good and can easily be misled (e.g., when one of the selections
         # had fewer atoms but the residues in mobile and reference have
         # each the same number)
-        try:
-            mass_mismatches = (np.absolute(ag1.masses - ag2.masses) > tol_mass)
-        except ValueError:
-            errmsg = ("Failed to find matching atoms: len(reference) = {}, len(mobile) = {} " +
-                    "Try to improve your selections for mobile and reference.").format(
-                        ag1.n_atoms, ag2.n_atoms)
-            logger.error(errmsg)
-            raise_from(SelectionError(errmsg), None)
-
-        if np.any(mass_mismatches):
-            # Test 2 failed.
-            # diagnostic output:
-            logger.error("Atoms: reference | trajectory")
-            for ar, at in zip(ag1[mass_mismatches], ag2[mass_mismatches]):
-                logger.error(
-                    "{0!s:>4} {1:3d} {2!s:>3} {3!s:>3} {4:6.3f}  |  {5!s:>4} {6:3d} {7!s:>3} {8!s:>3} {9:6.3f}".format(
-                        ar.segid,
-                        ar.resid,
-                        ar.resname,
-                        ar.name,
-                        ar.mass,
-                        at.segid,
-                        at.resid,
-                        at.resname,
-                        at.name,
-                        at.mass))
-            errmsg = ("Inconsistent selections, masses differ by more than {0}; "
-                    "mis-matching atoms are shown above.").format(tol_mass)
-            logger.error(errmsg)
-            raise SelectionError(errmsg)
+        if (not hasattr(ag1, 'masses') or not hasattr(ag2, 'masses')):
+            msg = "Atoms could not be matched since they don't contain masses."
+            logger.info(msg)
+            warnings.warn(msg, category=SelectionWarning)
+        else:
+            try:
+                mass_mismatches = (np.absolute(ag1.masses - ag2.masses) > tol_mass)
+            except ValueError:
+                errmsg = ("Failed to find matching atoms: len(reference) = {}, len(mobile) = {} "
+                          "Try to improve your selections for mobile and reference.").format(
+                            ag1.n_atoms, ag2.n_atoms)
+                logger.error(errmsg)
+                raise_from(SelectionError(errmsg), None)
+
+            if np.any(mass_mismatches):
+                # Test 2 failed.
+                # diagnostic output:
+                logger.error("Atoms: reference | trajectory")
+                for ar, at in zip(ag1[mass_mismatches], ag2[mass_mismatches]):
+                    logger.error(
+                        "{0!s:>4} {1:3d} {2!s:>3} {3!s:>3} {4:6.3f}  |  {5!s:>4} {6:3d} {7!s:>3} {8!s:>3} {9:6.3f}".format(
+                            ar.segid,
+                            ar.resid,
+                            ar.resname,
+                            ar.name,
+                            ar.mass,
+                            at.segid,
+                            at.resid,
+                            at.resname,
+                            at.name,
+                            at.mass))
+                errmsg = ("Inconsistent selections, masses differ by more than {0}; "
+                           "mis-matching atoms are shown above.").format(tol_mass)
+                logger.error(errmsg)
+                raise SelectionError(errmsg)
 
     return ag1, ag2

package/MDAnalysis/analysis/rms.py

@@ -416,7 +416,7 @@ def __init__(self, atomgroup, reference=None, select='all',
              length) or if it is not a 1D array (see
              :func:`MDAnalysis.lib.util.get_weights`).
 
-             A :exc:`ValueError` is also raised if the length of `weights_groupselections` 
+             A :exc:`ValueError` is also raised if the length of `weights_groupselections`
              are not compatible with `groupselections`.
 
         Notes
@@ -545,7 +545,7 @@ def __init__(self, atomgroup, reference=None, select='all',
         # check weights type
         if iterable(self.weights) and (np.array(weights).dtype
                                 not in (np.dtype('float64'),np.dtype('int64'))):
-            raise TypeError("weight should only be be 'mass', None or 1D float array."
+            raise TypeError("weights should only be 'mass', None or 1D float array."
                                  "For weights on groupselections, use **weight_groupselections** ")
         if iterable(self.weights) or self.weights != "mass":
             get_weights(self.mobile_atoms, self.weights)

package/MDAnalysis/lib/util.py

@@ -924,7 +924,7 @@ def get_ext(filename):
     ext : str
     """
     root, ext = os.path.splitext(filename)
-    
+
     if ext.startswith(os.extsep):
         ext = ext[1:]
 
@@ -1369,7 +1369,7 @@ def get_weights(atoms, weights):
                             "the atoms ({1})".format(
                                 len(weights), len(atoms)))
     elif weights is not None:
-        raise TypeError("weights must be {'mass', None} or an iterable of the "
+        raise ValueError("weights must be {'mass', None} or an iterable of the "
                         "same size as the atomgroup.")
 
     return weights
@@ -1581,7 +1581,7 @@ def unique_rows(arr, return_index=False):
     Examples
     --------
     Remove dupicate rows from an array:
-    
+
     >>> a = np.array([[0, 1], [1, 2], [1, 2], [0, 1], [2, 3]])
     >>> b = unique_rows(a)
     >>> b

package/MDAnalysis/transformations/fit.py

@@ -45,47 +45,47 @@
 
 def fit_translation(ag, reference, plane=None, weights=None):
 
-    """Translates a given AtomGroup so that its center of geometry/mass matches 
+    """Translates a given AtomGroup so that its center of geometry/mass matches
     the respective center of the given reference. A plane can be given by the
     user using the option `plane`, and will result in the removal of
     the translation motions of the AtomGroup over that particular plane.
-    
+
     Example
     -------
-    Removing the translations of a given AtomGroup `ag` on the XY plane by fitting 
+    Removing the translations of a given AtomGroup `ag` on the XY plane by fitting
     its center of mass to the center of mass of a reference `ref`:
-    
+
     .. code-block:: python
-    
+
         ag = u.select_atoms("protein")
         ref = mda.Universe("reference.pdb")
         transform = mda.transformations.fit_translation(ag, ref, plane="xy",
                                                         weights="mass")
         u.trajectory.add_transformations(transform)
-    
+
     Parameters
     ----------
     ag : Universe or AtomGroup
        structure to translate, a
-       :class:`~MDAnalysis.core.groups.AtomGroup` or a whole 
+       :class:`~MDAnalysis.core.groups.AtomGroup` or a whole
        :class:`~MDAnalysis.core.universe.Universe`
     reference : Universe or AtomGroup
-       reference structure, a :class:`~MDAnalysis.core.groups.AtomGroup` or a whole 
+       reference structure, a :class:`~MDAnalysis.core.groups.AtomGroup` or a whole
        :class:`~MDAnalysis.core.universe.Universe`
     plane: str, optional
-        used to define the plane on which the translations will be removed. Defined as a 
+        used to define the plane on which the translations will be removed. Defined as a
         string of the plane. Suported planes are yz, xz and xy planes.
     weights : {"mass", ``None``} or array_like, optional
        choose weights. With ``"mass"`` uses masses as weights; with ``None``
        weigh each atom equally. If a float array of the same length as
        `ag` is provided, use each element of the `array_like` as a
        weight for the corresponding atom in `ag`.
-    
+
     Returns
     -------
     MDAnalysis.coordinates.base.Timestep
     """
-    
+
     if plane is not None:
         axes = {'yz' : 0, 'xz' : 1, 'xy' : 2}
         try:
@@ -100,23 +100,19 @@ def fit_translation(ag, reference, plane=None, weights=None):
             AttributeError("{} or {} is not valid Universe/AtomGroup".format(ag,reference)),
             None)
     ref, mobile = align.get_matching_atoms(reference.atoms, ag.atoms)
-    try:
-        weights = align.get_weights(ref.atoms, weights=weights)
-    except (ValueError, TypeError):
-        raise_from(ValueError("weights must be {'mass', None} or an iterable of the "
-                        "same size as the atomgroup."), None)
-
-    ref_com = np.asarray(ref.center(weights), np.float32)
+    weights = align.get_weights(ref.atoms, weights=weights)
+    ref_com = ref.center(weights)
+    ref_coordinates = ref.atoms.positions - ref_com
 
     def wrapped(ts):
         mobile_com = np.asarray(mobile.atoms.center(weights), np.float32)
         vector = ref_com - mobile_com
         if plane is not None:
             vector[plane] = 0
         ts.positions += vector
-        
+
         return ts
-    
+
     return wrapped
 
 
@@ -125,44 +121,44 @@ def fit_rot_trans(ag, reference, plane=None, weights=None):
 
     Spatially align the group of atoms `ag` to `reference` by doing a RMSD
     fit.
-    
+
     This fit works as a way to remove translations and rotations of a given
     AtomGroup in a trajectory. A plane can be given using the flag `plane`
     so that only translations and rotations in that particular plane are
     removed. This is useful for protein-membrane systems to where the membrane
     must remain in the same orientation.
-    
+
     Example
     -------
     Removing the translations and rotations of a given AtomGroup `ag` on the XY plane
     by fitting it to a reference `ref`, using the masses as weights for the RMSD fit:
-    
+
     .. code-block:: python
-    
+
         ag = u.select_atoms("protein")
         ref = mda.Universe("reference.pdb")
-        transform = mda.transformations.fit_rot_trans(ag, ref, plane="xy", 
+        transform = mda.transformations.fit_rot_trans(ag, ref, plane="xy",
                                                       weights="mass")
         u.trajectory.add_transformations(transform)
- 
+
     Parameters
     ----------
     ag : Universe or AtomGroup
        structure to translate and rotate, a
-       :class:`~MDAnalysis.core.groups.AtomGroup` or a whole 
+       :class:`~MDAnalysis.core.groups.AtomGroup` or a whole
        :class:`~MDAnalysis.core.universe.Universe`
     reference : Universe or AtomGroup
-       reference structure, a :class:`~MDAnalysis.core.groups.AtomGroup` or a whole 
+       reference structure, a :class:`~MDAnalysis.core.groups.AtomGroup` or a whole
        :class:`~MDAnalysis.core.universe.Universe`
     plane: str, optional
-        used to define the plane on which the rotations and translations will be removed. 
+        used to define the plane on which the rotations and translations will be removed.
         Defined as a string of the plane. Supported planes are "yz", "xz" and "xy" planes.
     weights : {"mass", ``None``} or array_like, optional
        choose weights. With ``"mass"`` uses masses as weights; with ``None``
        weigh each atom equally. If a float array of the same length as
        `ag` is provided, use each element of the `array_like` as a
        weight for the corresponding atom in `ag`.
-       
+
     Returns
     -------
     MDAnalysis.coordinates.base.Timestep
@@ -179,14 +175,10 @@ def fit_rot_trans(ag, reference, plane=None, weights=None):
     except AttributeError:
         raise_from(AttributeError("{} or {} is not valid Universe/AtomGroup".format(ag,reference)), None)
     ref, mobile = align.get_matching_atoms(reference.atoms, ag.atoms)
-    try:
-        weights = align.get_weights(ref.atoms, weights=weights)
-    except (ValueError, TypeError):
-        raise_from(ValueError("weights must be {'mass', None} or an iterable of the "
-                        "same size as the atomgroup."), None)
+    weights = align.get_weights(ref.atoms, weights=weights)
     ref_com = ref.center(weights)
     ref_coordinates = ref.atoms.positions - ref_com
-    
+
     def wrapped(ts):
         mobile_com = mobile.atoms.center(weights)
         mobile_coordinates = mobile.atoms.positions - mobile_com
@@ -203,7 +195,7 @@ def wrapped(ts):
         ts.positions = ts.positions - mobile_com
         ts.positions = np.dot(ts.positions, rotation.T)
         ts.positions = ts.positions + vector
-        
+
         return ts
-    
+
     return wrapped