search

channotation / chap

CHAP is a tool for the functional annotation of ion channel structures:
http://www.channotation.org
Other
18 stars 10 forks source link

Patches for fixing a few issues and bugs #22

Open Fravadona opened 4 years ago

Fravadona commented 4 years ago

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 commented 4 years ago

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!