Merge pull request #257 from rmcolq/refactoring_asserts

Refactoring asserts

.dockerignore

@@ -20,4 +20,7 @@ tmp/
 cmake_install.cmake
 compile_commands.json
 Makefile
-pandora.cbp
+pandora.cbp
+build_portable_executable
+pandora-linux-precompiled
+/cmake-build-release/

.gitignore

@@ -115,5 +115,6 @@ example/pandora_workflow
 
 #portable binary build dir
 build_portable_executable
-
 pandora-linux-precompiled
+
+/cmake-build-release/

.gitmodules

@@ -1,3 +1,6 @@
-[submodule "cgranges"]
-	path = cgranges
+[submodule "thirdparty/gatb-core"]
+	path = thirdparty/gatb-core
+	url = https://github.com/leoisl/gatb-core
+[submodule "thirdparty/cgranges"]
+	path = thirdparty/cgranges
 	url = https://github.com/lh3/cgranges

.travis.yml

@@ -19,7 +19,6 @@ include:
       apt:
         packages:
           - clang-format-8
-          - zlib1g-dev
 
 jobs:
   fast_finish: true
@@ -30,7 +29,7 @@ jobs:
         - if [ $? -ne 1 ]; then echo "Not all source and header files are formatted with clang-format"; exit 1; fi
     - stage: "Build and Test"
       env:
-        - BUILD_TYPE="Debug"
+        - BUILD_TYPE="Release"
       script: bash ci/script.sh
 
 stages:

CHANGELOG.md

@@ -7,7 +7,33 @@ The format is based on
 project adheres to
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [Unreleased]
+## [v0.8.0]
+
+Improvements to the build process and performance.
+
+### Added
+- We now provide a script to build a portable precompiled binary as another option to run `pandora` easily.
+  The portable binary is now provided with the release;
+- `pandora` can now provide a meaningful stack trace in case of errors, to facilitate debugging
+  (need to pass flag `-DPRINT_STACKTRACE` to `CMake`). Due to this, we now add debug symbols (`-g` flag)
+  to every `pandora` build type, but this [does not impact performance](https://stackoverflow.com/a/39223245).
+  The precompiled binary has this enabled.
+
+### Changed
+- We now use the [Hunter](https://github.com/cpp-pm/hunter) package manager, removing the requirement of having `ZLIB` and
+  `Boost` system-wide installations;
+- `GATB` is now a git submodule instead of an external project downloaded and compiled during compilation time.
+  This means that when git cloning `pandora`, `cgranges` and `GATB` are also downloaded/cloned, and when preparing
+  the build (running `cmake`), `Hunter` downloads and installs `Boost`, `GTest` and `ZLIB`.
+  Thus we still need internet connection to prepare the build (running `cmake`) but not for compiling (running `make`).
+- We now use a GATB fork that accepts a `ZLIB` custom installation;
+- Refactored all thirdparty libraries (`cgranges`, `GATB`, `backward`, `CLI11`, `inthash`) into their own directory `thirdparty`.
+
+### Fixed
+- Refactored asserts into exceptions, and now `pandora` can be compiled correctly in the `Release` mode. 
+  The build process is thus able to create a more optimized binary, resulting in improved performance.
+
+
 
 ## [v0.7.0]
 

CMakeLists.txt

@@ -1,6 +1,7 @@
 cmake_minimum_required(VERSION 3.12)  # required by hunter ZLIB installation
 
 # include hunter
+set(HUNTER_ROOT ${CMAKE_BINARY_DIR}/hunter)
 include("cmake/HunterGate.cmake")
 HunterGate(
         URL "https://github.com/cpp-pm/hunter/archive/v0.23.289.tar.gz"
@@ -10,12 +11,18 @@ HunterGate(
 
 # project configuration
 set(PROJECT_NAME_STR pandora)
-project(${PROJECT_NAME_STR} VERSION "0.7.0" LANGUAGES C CXX)
+project(${PROJECT_NAME_STR} VERSION "0.8.0" LANGUAGES C CXX)
 configure_file( include/version.h.in ${CMAKE_BINARY_DIR}/include/version.h )
 
+# add or not feature to print the stack trace
+if(PRINT_STACKTRACE)
+    message(STATUS "Printing meaningful stacktrace enabled, please have binutils-dev installed")
+    add_compile_definitions(BACKWARD_HAS_BFD=1)
+    set(BACKWARD_LIBRARIES "-lbfd")
+else()
+    set(BACKWARD_LIBRARIES "")
+endif()
 
-# add a RELEASE_WITH_ASSERTS build type - TODO: FIX THIS
-set(CMAKE_CXX_FLAGS_RELEASE_WITH_ASSERTS "-O3")
 
 # C++11 required
 include(CheckCXXCompilerFlag)
@@ -32,6 +39,12 @@ endif ()
 # default flags
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DBOOST_SYSTEM_NO_DEPRECATED -Wall -Wextra")
 
+if(PRINT_STACKTRACE)
+    # -g is to add debug symbols, to make backtraces meaningful.
+    # it does not impact performance (see https://stackoverflow.com/a/39223245)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g")
+endif ()
+
 # compile with openmp only on Linux
 if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
     set(MACOSX TRUE)
@@ -58,7 +71,7 @@ set(ZLIB_LIBRARY ZLIB::ZLIB)
 # INSTALL GATB
 include(${PROJECT_SOURCE_DIR}/ext/gatb.cmake)
 include_directories(SYSTEM
-        ${gatb_source_dir}/gatb-core/src
+        ${gatb_source_dir}/src
         ${gatb_binary_dir}/include
         )
 link_directories(${gatb_binary_dir}/lib)
@@ -94,13 +107,15 @@ set(Boost_USE_STATIC_LIBS ON)
 #include directories as SYSTEM includes, thus warnings will be ignored for these
 include_directories(SYSTEM
         ${CMAKE_BINARY_DIR}/include
-        ${PROJECT_SOURCE_DIR}/cgranges/cpp
+        ${PROJECT_SOURCE_DIR}/thirdparty/cgranges/cpp
         )
 
 # normal includes: warnings will be reported for these
 include_directories(
         ${PROJECT_SOURCE_DIR}/include
         ${PROJECT_SOURCE_DIR}/src
+        ${PROJECT_SOURCE_DIR}/thirdparty/include
+        ${PROJECT_SOURCE_DIR}/thirdparty/src
 )
 
 file(GLOB_RECURSE SRC_FILES
@@ -111,6 +126,9 @@ file(GLOB_RECURSE SRC_FILES
         ${PROJECT_SOURCE_DIR}/include/*.h
         ${PROJECT_SOURCE_DIR}/include/*/*.hpp
         ${PROJECT_SOURCE_DIR}/include/*/*.h
+        ${PROJECT_SOURCE_DIR}/thirdparty/src/*.cpp
+        ${PROJECT_SOURCE_DIR}/thirdparty/include/*.hpp
+        ${PROJECT_SOURCE_DIR}/thirdparty/include/*.h
         )
 
 add_executable(${PROJECT_NAME} ${SRC_FILES})
@@ -122,6 +140,7 @@ target_link_libraries(${PROJECT_NAME}
         ${ZLIB_LIBRARY}
         ${CMAKE_DL_LIBS}
         ${STATIC_C_CXX}
+        ${BACKWARD_LIBRARIES}
 )
 
 enable_testing()

Dockerfile

@@ -19,7 +19,7 @@ RUN apt update \
 #============================================
 # can override the build type with docker's --build-arg command
 # https://docs.docker.com/engine/reference/builder/#arg
-ARG PANDORA_BUILD_TYPE="RELEASE_WITH_ASSERTS"
+ARG PANDORA_BUILD_TYPE="Release"
 ENV PANDORA_DIR "/pandora/"
 
 COPY . $PANDORA_DIR

README.md

@@ -83,13 +83,12 @@ can be found [here](https://gcc.gnu.org/onlinedocs/gfortran/OpenMP.html).
 
 * **Download**:
   ```
-  wget "https://www.dropbox.com/s/74ptrnk4k5qcc6o/pandora-linux-precompiled_v0.8.0_beta?dl=1" -O pandora-linux-precompiled_v0.8.0_beta
+  wget https://github.com/rmcolq/pandora/releases/download/v0.8.0-alpha/pandora-linux-precompiled-v0.8.0-alpha
   ```
-  * **TODO: updated to a github link when we make the release;**
 * **Running**:
 ```
-chmod +x pandora-linux-precompiled_v0.8.0_beta
-./pandora-linux-precompiled_v0.8.0_beta -h
+chmod +x pandora-linux-precompiled-v0.8.0-alpha
+./pandora-linux-precompiled-v0.8.0-alpha -h
 ```
 
 * **Compatibility**: This precompiled binary works on pretty much any glibc-2.12-or-later-based x86 and x86-64 Linux distribution 
@@ -110,7 +109,7 @@ chmod +x pandora-linux-precompiled_v0.8.0_beta
 
 ![Docker Cloud Build Status](https://img.shields.io/docker/cloud/build/rmcolq/pandora)
 
-We highly recommend that you download a containerized image of Pandora.
+You can also download a containerized image of Pandora.
 Pandora is hosted on Dockerhub and images can be downloaded with the
 command:
 
@@ -128,6 +127,8 @@ NB For consistency, we no longer maintain images on singularity hub.
 
 ### Installation from source
 
+This is the hardest way to install `pandora`, but that yields the most optimised binary.
+
 Requirements:
 - A Unix or Mac OS, with a C++11 compiler toolset (e.g. `g++`, `ld`, `make`, `ctest`, etc), `cmake`, `git` and `wget`.
 
@@ -138,11 +139,14 @@ git clone --single-branch https://github.com/rmcolq/pandora.git --recursive
 cd pandora
 mkdir -p build
 cd build
-cmake ..
+cmake -DCMAKE_BUILD_TYPE=Release .. 
 make -j4
 ctest -VV
 ```
 
+* If you want to produce meaningful stack traces in case `pandora` errors out, `binutils-dev` must be installed and the
+  `cmake` must receive this additional parameter: `-DPRINT_STACKTRACE=True`.
+
 ## Usage
 
 See [Usage](https://github.com/rmcolq/pandora/wiki/Usage).

cmake/Hunter/config.cmake

@@ -2,4 +2,14 @@ hunter_config(
     Boost
     VERSION
     "1.62.0"
+    CONFIGURATION_TYPES
+    Release
+)
+
+hunter_config(
+    GTest
+    VERSION
+    "1.10.0"
+    CONFIGURATION_TYPES
+    Release
 )

example/README.md

@@ -15,7 +15,7 @@ pandora_workflow_data/ : contains other input and configuration files to run the
 
 ### Dependencies
 
-* [`singularity`](https://sylabs.io/)
+* `md5sum`, `wget`, `GCC` 4.9+ (see [why](../README.md#no-installation-needed---precompiled-portable-binary)).
 
 ### Running
 ```
@@ -41,9 +41,7 @@ We can see samples `toy_sample_1` and `toy_sample_2` genotype towards different
 
 ### Dependencies
 
-* [`singularity`](https://sylabs.io/)
-* `git`
-* `python 3.6+`
+* [`singularity`](https://sylabs.io/), `git`, `python 3.6+`
 
 ### Running
 ```

example/pandora-linux-precompiled-v0.8.0-alpha.md5sum.txt

@@ -0,0 +1 @@
+0703e724c62cfe41c048519f53298825  pandora-linux-precompiled-v0.8.0-alpha

example/run_pandora_nodenovo.sh

@@ -1,4 +1,22 @@
 #!/usr/bin/env bash
 set -eu
-pandora_command="pandora index prgs/toy_prg.fa && pandora compare --genotype -o output_toy_example_no_denovo prgs/toy_prg.fa reads/read_index.tsv"
-singularity exec docker://rmcolq/pandora:latest bash -c "${pandora_command}"
+
+# configs
+pandora_URL="https://github.com/rmcolq/pandora/releases/download/v0.8.0-alpha/pandora-linux-precompiled-v0.8.0-alpha"
+pandora_executable="./pandora-linux-precompiled-v0.8.0-alpha"
+pandora_md5sum_file="./pandora-linux-precompiled-v0.8.0-alpha.md5sum.txt"
+
+
+if md5sum -c "${pandora_md5sum_file}"; then
+    # The MD5 sum match
+    echo "${pandora_executable} has correct MD5 sum, proceeding..."
+else
+    # The MD5 sum didn't match
+    echo "${pandora_executable} does not exist or does not have correct MD5 sum, downloading..."
+    wget "${pandora_URL}" -O "${pandora_executable}"
+    chmod +x "${pandora_executable}"
+fi
+
+"${pandora_executable}" index prgs/toy_prg.fa
+"${pandora_executable}" compare --genotype -o output_toy_example_no_denovo prgs/toy_prg.fa reads/read_index.tsv
+

ext/gatb.cmake

@@ -6,10 +6,8 @@ SET (GATB_CORE_EXCLUDE_TESTS     1)
 SET (GATB_CORE_INCLUDE_EXAMPLES  1)
 
 ExternalProject_Add(gatb
-        GIT_REPOSITORY https://github.com/leoisl/gatb-core
-        GIT_TAG "1.4.1_zlib"
+        SOURCE_DIR "${CMAKE_SOURCE_DIR}/thirdparty/gatb-core/gatb-core"
         PREFIX "${CMAKE_CURRENT_BINARY_DIR}/gatb"
-        SOURCE_SUBDIR gatb-core
         CMAKE_ARGS -DKSIZE_LIST=32 -DZLIB_ROOT=${ZLIB_ROOT}
         INSTALL_COMMAND "")
 

include/Maths.h

@@ -35,7 +35,7 @@ class Maths {
             typename std::iterator_traits<Iterator>::difference_type difference_type;
 
         difference_type number_of_elements = std::distance(begin, end);
-        bool no_elements_in_container = number_of_elements == 0;
+        const bool no_elements_in_container = number_of_elements == 0;
         if (no_elements_in_container) {
             return get_default_value<Iterator>();
         }

include/OptionsAggregator.h

@@ -1,6 +1,9 @@
 #ifndef PANDORA_OPTIONSAGGREGATOR_H
 #define PANDORA_OPTIONSAGGREGATOR_H
 
+#include <vector>
+#include <cstdint>
+
 class GenotypingOptions {
 private:
     std::vector<uint32_t> sample_index_to_exp_depth_covg;

include/compare_main.h

@@ -10,7 +10,6 @@
 #include <fstream>
 #include <algorithm>
 #include <map>
-#include <cassert>
 #include <boost/log/trivial.hpp>
 #include <boost/log/expressions.hpp>
 
@@ -24,6 +23,7 @@
 #include "estimate_parameters.h"
 #include "OptionsAggregator.h"
 #include "CLI11.hpp"
+#include "fatal_error.h"
 
 using std::set;
 using std::vector;

include/denovo_discovery/denovo_utils.h

@@ -4,7 +4,6 @@
 #include <set>
 #include <algorithm>
 #include <memory>
-#include <cassert>
 #include <utility>
 #include <vector>
 #include "localnode.h"

include/estimate_parameters.h

@@ -5,6 +5,7 @@
 #include <cstring>
 #include <cstdint>
 #include <boost/filesystem.hpp>
+#include "fatal_error.h"
 
 namespace fs = boost::filesystem;
 

include/fastaq.h

@@ -3,17 +3,15 @@
 
 #include <vector>
 #include <iostream>
-#include <cassert>
 #include <cctype>
-#include <unordered_map>
-
 #include <unordered_map>
 #include <iostream>
 #include <cmath>
 #include <boost/filesystem.hpp>
 #include <boost/iostreams/filtering_streambuf.hpp>
 #include <boost/iostreams/filter/gzip.hpp>
 #include <boost/log/trivial.hpp>
+#include "fatal_error.h"
 
 namespace fs = boost::filesystem;