Merge branch 'devel' into fix_130

Signed-off-by: Christopher Woods <[email protected]>

corelib/src/libs/SireVol/triclinicbox.cpp

@@ -41,16 +41,6 @@
 #include "SireError/errors.h"
 #include "SireStream/datastream.h"
 
-// Helper struct for sorting based on first pair value.
-struct QPairFirstComparer
-{
-    template <typename T1, typename T2>
-    bool operator()(const QPair<T1, T2> &a, const QPair<T1, T2> &b) const
-    {
-        return a.first < b.first;
-    }
-};
-
 using namespace SireVol;
 using namespace SireBase;
 using namespace SireMaths;
@@ -61,10 +51,10 @@ static const RegisterMetaType<TriclinicBox> r_box;
 /** Serialise to a binary datastream */
 QDataStream &operator<<(QDataStream &ds, const TriclinicBox &box)
 {
-    writeHeader(ds, r_box, 1) << box.v0_orig << box.v1_orig << box.v2_orig;
-
-    // no need to store anything else as it can be regenerated
-
+    writeHeader(ds, r_box, 2) << box.v0 << box.v1 << box.v2
+                              << box.rotation_matrix << box.cell_matrix << box.cell_matrix_inverse
+                              << box.dist_max << box._alpha << box._beta << box._gamma << box.vol
+                              << box.is_rotated << box.is_reduced << box.invlength;
     return ds;
 }
 
@@ -76,15 +66,19 @@ QDataStream &operator>>(QDataStream &ds, TriclinicBox &box)
     if (v == 1)
     {
         Vector v0, v1, v2;
-
-        // Load in the original box vectors.
         ds >> v0 >> v1 >> v2;
 
         // Reconstruct the box.
-        box = TriclinicBox(v0, v1, v2);
+        box = TriclinicBox(v0, v1, v2, true, true);
+    }
+    else if (v == 2)
+    {
+        ds >> box.v0 >> box.v1 >> box.v2 >> box.rotation_matrix >> box.cell_matrix >> box.cell_matrix_inverse
+           >> box.dist_max >> box._alpha >> box._beta >> box._gamma >> box.vol >> box.is_rotated >> box.is_reduced
+           >> box.invlength;
     }
     else
-        throw version_error(v, "1", r_box, CODELOC);
+        throw version_error(v, "1,2", r_box, CODELOC);
 
     return ds;
 }
@@ -101,7 +95,7 @@ TriclinicBox::TriclinicBox() : ConcreteProperty<TriclinicBox, Cartesian>()
 
 /** Construct a TriclinicBox with the specified lattice vectors */
 TriclinicBox::TriclinicBox(const Vector &v0, const Vector &v1, const Vector &v2, bool auto_rotate, bool auto_reduce)
-    : ConcreteProperty<TriclinicBox, Cartesian>(), v0(v0), v1(v1), v2(v2), v0_orig(v0), v1_orig(v1), v2_orig(v2)
+    : ConcreteProperty<TriclinicBox, Cartesian>(), v0(v0), v1(v1), v2(v2)
 {
     this->construct(v0, v1, v2, auto_rotate, auto_reduce);
 }
@@ -142,12 +136,6 @@ TriclinicBox::TriclinicBox(double a, double b, double c, const SireUnits::Dimens
 /** Construct a TriclinicBox with the specified lattice vectors */
 void TriclinicBox::construct(const Vector &v0, const Vector &v1, const Vector &v2, bool auto_rotate, bool auto_reduce)
 {
-    // Store the original lattice vectors. These are needed for streaming
-    // support.
-    this->v0_orig = v0;
-    this->v1_orig = v1;
-    this->v2_orig = v2;
-
     // Set the initial lattice vectors.
     this->v0 = v0;
     this->v1 = v1;
@@ -375,8 +363,8 @@ void TriclinicBox::setAttributes()
 /** Copy constructor */
 TriclinicBox::TriclinicBox(const TriclinicBox &other)
     : ConcreteProperty<TriclinicBox, Cartesian>(other), v0(other.v0), v1(other.v1), v2(other.v2),
-      v0_orig(other.v0_orig), v1_orig(other.v1_orig), v2_orig(other.v2_orig), rotation_matrix(other.rotation_matrix),
-      cell_matrix(other.cell_matrix), cell_matrix_inverse(other.cell_matrix_inverse), M(other.M),
+      rotation_matrix(other.rotation_matrix), cell_matrix(other.cell_matrix),
+      cell_matrix_inverse(other.cell_matrix_inverse), M(other.M),
       dist_max(other.dist_max), max_length(other.max_length), _alpha(other._alpha), _beta(other._beta),
       _gamma(other._gamma), vol(other.vol), is_rotated(other.is_rotated), is_reduced(other.is_reduced), invlength(other.invlength)
 {
@@ -395,9 +383,6 @@ TriclinicBox &TriclinicBox::operator=(const TriclinicBox &other)
         v0 = other.v0;
         v1 = other.v1;
         v2 = other.v2;
-        v0_orig = other.v0_orig;
-        v1_orig = other.v1_orig;
-        v2_orig = other.v2_orig;
         rotation_matrix = other.rotation_matrix;
         cell_matrix = other.cell_matrix;
         dist_max = other.dist_max;
@@ -418,15 +403,26 @@ TriclinicBox &TriclinicBox::operator=(const TriclinicBox &other)
 /** Comparison operator */
 bool TriclinicBox::operator==(const TriclinicBox &other) const
 {
-    return v0 == other.v0 and v1 == other.v1 and v2 == other.v2 and v0_orig == other.v0_orig and
-           v1_orig == other.v1_orig and v2_orig == other.v2_orig;
+    return v0 == other.v0 and
+           v1 == other.v1 and
+           v2 == other.v2 and
+           rotation_matrix == other.rotation_matrix and
+           cell_matrix == other.cell_matrix and
+           cell_matrix_inverse == other.cell_matrix_inverse and
+           dist_max == other.dist_max and
+           _alpha == other._alpha and
+           _beta == other._beta and
+           _gamma == other._gamma and
+           vol == other.vol and
+           is_rotated == other.is_rotated and
+           is_reduced == other.is_reduced and
+           invlength == other.invlength;
 }
 
 /** Comparison operator */
 bool TriclinicBox::operator!=(const TriclinicBox &other) const
 {
-    return v0 != other.v0 or v1 != other.v1 or v2 != other.v2 or v0_orig != other.v0_orig or v1_orig != other.v1_orig or
-           v2_orig != other.v2_orig;
+    return !(*this == other);
 }
 
 /** A Triclinic box is periodic! */

corelib/src/libs/SireVol/triclinicbox.h

@@ -308,15 +308,6 @@ namespace SireVol
         /** The third box vector */
         Vector v2;
 
-        /** The first box vector (original copy) */
-        Vector v0_orig;
-
-        /** The second box vector (original copy) */
-        Vector v1_orig;
-
-        /** The third box vector (original copy) */
-        Vector v2_orig;
-
         /** The rotation matrix used to transform the box to meet the requirements
             of molecular dynamics engines.
           */

doc/source/changelog.rst

@@ -51,6 +51,8 @@ organisation on `GitHub <https://github.com/openbiosim/sire>`__.
 * Fix validation of ``perturbable_constraint`` dynamics option when the string
   includes hyphens. This fixes issue #130.
 
+* Fix streaming of :cls:`~sire.vol.TriclinicBox` objects. This fixes issue #128.
+
 * Please add an item to this changelog when you create your PR
 
 `2023.4.1 <https://github.com/openbiosim/sire/compare/2023.4.0...2023.4.1>`__ - October 2023

tests/vol/test_triclinic.py

@@ -113,3 +113,31 @@ def test_cresset_box():
 
     # Try to set the periodic box vectors using those from the inpcrd file.
     topology.setPeriodicBoxVectors(inpcrd.boxVectors)
+
+
+def test_stream():
+    """
+    Test that a rotated and reduced TriclinicBox is correctly recovered when
+    streaming to and from a file.
+    """
+
+    # Create a temporary working directory.
+    tmp_dir = tempfile.TemporaryDirectory()
+    tmp_path = tmp_dir.name
+    s3_file = f"{tmp_path}/box.s3"
+
+    # Create a triclinic box.
+    box = sr.vol.TriclinicBox.truncated_octahedron(1.0, True, True)
+
+    # Make sure the box has been rotated and reduced.
+    assert box.is_rotated()
+    assert box.is_reduced()
+
+    # Stream to file.
+    sr.stream.save(box, s3_file)
+
+    # Load the file.
+    recovered_box = sr.stream.load(s3_file)
+
+    # Make sure the boxes are the same.
+    assert recovered_box == box