Patches for fixing a few issues and bugs

[Fravadona]

Hi Inniag,

When trying to install CHAP on CentOS 7 linux (with GROMACS 2018.8, Intel Compilers 2018.3 and Intel MKL 2018.3) I had to fix a few issues and bugs for making it work correctly.

1. The first problem was during compilation because the C++11 standard doesn't like implicit "narrowing conversions" (and the Intel compilers fail because of that); an obvious fix is to cast the conversions explicitly, here's the patch:

--- chap-version_0_9_1/src/trajectory-analysis/chap_trajectory_analysis.cpp.orig	2018-05-02 13:22:36.000000000 +0200
+++ chap-version_0_9_1/src/trajectory-analysis/chap_trajectory_analysis.cpp	2019-12-09 13:52:36.745685228 +0100
@@ -84,7 +84,7 @@
     frameStreamData_.setMultipoint(true); 
 
     // default initial probe position and chanell direction:
-    pfInitProbePos_ = {std::nan(""), std::nan(""), std::nan("")};
+    pfInitProbePos_ = {(real)std::nan(""), (real)std::nan(""), (real)std::nan("")};
     pfChanDirVec_ = {0.0, 0.0, 1.0};
 }
 
--- chap-version_0_9_1/test/geometry/ut_bspline_basis_set.cpp.orig	2018-05-02 13:22:36.000000000 +0200
+++ chap-version_0_9_1/test/geometry/ut_bspline_basis_set.cpp	2019-12-06 16:57:13.304949817 +0100
@@ -41,7 +41,7 @@
         std::vector<real> uniqueKnots_ = {-4, -0.5, 0.0, 0.5, 4};
          
         // evaluation points:
-        std::vector<real> evalPoints_ = {-4.0, -2.5, 0.0, 0.5, -1.0, std::sqrt(2.0), 4.0};
+        std::vector<real> evalPoints_ = {-4.0, -2.5, 0.0, 0.5, -1.0, (real)std::sqrt(2.0), 4.0};
 
         // true knot span index:
         std::vector<size_t> knotSpanIdx_ = {0, 0, 2, 3, 0, 3, 3};
--- chap-version_0_9_1/test/statistics/ut_amise_optimal_bandwidth_estimator.cpp.orig	2018-05-02 13:22:36.000000000 +0200
+++ chap-version_0_9_1/test/statistics/ut_amise_optimal_bandwidth_estimator.cpp	2019-12-06 16:59:40.880202641 +0100
@@ -62,7 +62,7 @@
 
     // parameters:
     int numReps = 2;
-    std::vector<real> mean = {-10, 0.0, std::sqrt(2.0)};
+    std::vector<real> mean = {-10, 0.0, (real)std::sqrt(2.0)};
     std::vector<real> numSamples = {250, 500, 750};
     std::vector<real> standardDeviation = {100.0, 1.0, 0.01};
 
--- chap-version_0_9_1/test/statistics/ut_histogram_density_estimator.cpp.orig	2018-05-02 13:22:36.000000000 +0200
+++ chap-version_0_9_1/test/statistics/ut_histogram_density_estimator.cpp	2019-12-06 17:07:42.104516814 +0100
@@ -82,7 +82,7 @@
     HistogramDensityEstimator hde;
 
     // some bin widths to try:
-    std::vector<real> binWidth = {1.0, 0.1, 1e-5, std::sqrt(2.0)};
+    std::vector<real> binWidth = {1.0, 0.1, 1e-5, (real)std::sqrt(2.0)};
 
     // loop over bin widths and check that breaks are correct:
     for(auto it = binWidth.begin(); it != binWidth.end(); it++)
@@ -138,7 +138,7 @@
 
     // some bin widths to try:
     // NOTE: if bin width too small, spline interpolation will fail!
-    std::vector<real> binWidth = {1.0, 0.1, 1e-2, 0.1*std::sqrt(2.0)};
+    std::vector<real> binWidth = {1.0, 0.1, 1e-2, (real)(0.1*std::sqrt(2.0))};
 
     // perform tests for each bin width value:
     for(auto bw : binWidth)
@@ -187,7 +187,7 @@
 
     // some bin widths to try:
     // NOTE: if bin width too small, spline interpolation will fail!
-    std::vector<real> binWidth = {1.0, 0.1, 1e-2, 0.1*std::sqrt(2.0)};
+    std::vector<real> binWidth = {1.0, 0.1, 1e-2, (real)(0.1*std::sqrt(2.0))};
 
     // perform tests for each binwidth value:
     for(auto bw : binWidth)
@@ -238,8 +238,8 @@
 
     // some bin widths to try:
     // NOTE: if bin width too small, spline interpolation will fail!
-    std::vector<real> binWidth = {1.0, 0.1, 1e-2, 0.1*std::sqrt(2.0)};
-    binWidth = {1.0, 0.1, 0.01, 0.1*std::sqrt(2.0)};
+    std::vector<real> binWidth = {1.0, 0.1, 1e-2, (real)(0.1*std::sqrt(2.0))};
+    binWidth = {1.0, 0.1, 0.01, (real)(0.1*std::sqrt(2.0))};
 
     // get data range:
     real dataMin = *std::min_element(testData_.begin(), testData_.end());
@@ -268,7 +268,7 @@
                     0);
 
             // this should always be zero:
-            ASSERT_NEAR(0.0, density, std::sqrt(eps));
+            ASSERT_NEAR(0.0, density, (real)std::sqrt(eps));
         }
  
         // evaluate density below data range:
@@ -283,7 +283,7 @@
                     0);
 
             // this should always be zero:
-            ASSERT_NEAR(0.0, density, std::sqrt(eps));
+            ASSERT_NEAR(0.0, density, (real)std::sqrt(eps));
         }
 
         // evaluate spline within data range:
@@ -317,7 +317,7 @@
             integral += d;
         }
         integral *= evalStep;
-        ASSERT_NEAR(1.0, integral, std::sqrt(eps));
+        ASSERT_NEAR(1.0, integral, (real)std::sqrt(eps));
    }
 }
 
--- chap-version_0_9_1/test/statistics/ut_summary_statistics.cpp.orig	2018-05-02 13:22:36.000000000 +0200
+++ chap-version_0_9_1/test/statistics/ut_summary_statistics.cpp	2019-12-06 17:16:08.457527879 +0100
@@ -44,7 +44,7 @@
         // constructor for creating test data:
         SummaryStatisticsTest()
         {
-            testData_ = {0.3, 1.5, -0.9, std::sqrt(2.0), -5.1};
+            testData_ = {0.3, 1.5, -0.9, (real)std::sqrt(2.0), -5.1};
         }
 
     
--- chap-version_0_9_1/test/statistics/ut_gaussian_density_derivative.cpp.orig	2018-05-02 13:22:36.000000000 +0200
+++ chap-version_0_9_1/test/statistics/ut_gaussian_density_derivative.cpp	2019-12-06 17:24:42.659436050 +0100
@@ -263,9 +263,9 @@
 
                 // check that truncation criterion is met:
                 real b = std::min<real>(gdd.rc_, 
-                                  0.5*(gdd.ri_ + std::sqrt(gdd.ri_*gdd.ri_ + 
+                                  0.5*(gdd.ri_ + (real)std::sqrt(gdd.ri_*gdd.ri_ + 
                                   8.0*trunc*gdd.bw_*gdd.bw_)));
-                real error = std::sqrt(gdd.factorial(gdd.r_))
+                real error = (real)std::sqrt(gdd.factorial(gdd.r_))
                            / gdd.factorial(trunc)
                            * std::pow((gdd.ri_ * b / (gdd.bw_*gdd.bw_)), trunc)
                            * std::exp(-(std::pow(gdd.ri_ - b, 2))
--- chap-version_0_9_1/test/geometry/ut_basis_spline.cpp.orig	2018-05-02 13:22:36.000000000 +0200
+++ chap-version_0_9_1/test/geometry/ut_basis_spline.cpp	2019-12-10 18:55:53.114400365 +0100
@@ -49,7 +49,7 @@
         std::vector<real> knotVector_ = {-4, -0.5, 0.0, 0.5, 4};
          
         // evaluation points:
-        std::vector<real> evalPoints_ = {-4.0, -2.5, 0.0, 0.5, -1.0, std::sqrt(2.0), 4.0};
+        std::vector<real> evalPoints_ = {-4.0, -2.5, 0.0, 0.5, -1.0, (real)std::sqrt(2.0), 4.0};
 };
 
 

2. The second problem cropped up during the execution of the annotation examples. The JSON "stream_" temporary files were incorrectly generated (empty) thus aborting the job. In the code I found that the file is opened in appending READ mode instead of appending WRITE mode; here's the patch:

--- chap-version_0_9_1/src/io/analysis_data_json_frame_exporter.cpp.orig	2018-05-02 13:22:36.000000000 +0200
+++ chap-version_0_9_1/src/io/analysis_data_json_frame_exporter.cpp	2019-12-10 18:45:48.496459218 +0100
@@ -169,7 +169,7 @@
         const gmx::AnalysisDataFrameHeader& /*frame*/)
 {
     // open output file separately for each frame:
-    file_.open(fileName_.c_str(), std::fstream::app);
+    file_.open(fileName_.c_str(), std::fstream::out | std::fstream::app);
 
     // create buffer writer string writer:
     rapidjson::StringBuffer buffer;

3. The last problem was more tricky to debug and I'm not very sure about the correctness of my fix. It seams to be a rounding problem during the generation of the Wavefront Object file because the values were very close to zero but negatives and thus didn't pass the sanity check. My fix is to do the scaling with "double" instead of "real"; here's the patch:

--- chap-version_0_9_1/src/io/molecular_path_obj_exporter.cpp.orig	2018-05-02 13:22:36.000000000 +0200
+++ chap-version_0_9_1/src/io/molecular_path_obj_exporter.cpp	2019-12-10 21:33:58.590351428 +0100
@@ -1064,7 +1064,7 @@
         // for sequential colour scale, shift data to positive real range and
         // scale by length of data interval:
         real shift = -minProp;
-        real scale = 1.0/(maxProp - minProp);  
+        double scale = 1.0/(maxProp - minProp);  
        
         // shift and scale the property array:
         std::for_each(prop.begin(), prop.end(), [shift](real &p){p += shift;});
@@ -1075,7 +1075,7 @@
         // for divergent colour scale, scale both positive and negative values
         // such that data lies in [-0.5, 0.5], then shift by 0.5:
         real shift = 0.5;
-        real scale = 1.0/std::max(std::fabs(minProp), std::fabs(maxProp))/2.0; 
+        double scale = 1.0/std::max(std::fabs(minProp), std::fabs(maxProp))/2.0; 
 
         // scale and shift:
         std::for_each(prop.begin(), prop.end(), [scale](real &p){p *= scale;});

That is all for the bug fixes,
Cheers.

PS: patch file with all the previous fixes inside :
version_0_9_1_fixes.patch.txt

[Inniag]

Hi Fravadona,
Thank you for your work, this is a really valuable contribution that seems to fix many problems other users already reported! Unfortunately, I am tied up in some other tasks at the moment and won't be able to look at this in much detail for the next two weeks. Over the holidays I'll hopefully have some time to integrate your fixes (and the enhancements in #23) into CHAP. I totally appreciate your help on this project!
Best wishes!