Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bayestyper: add arm64/aarch64 builds #52076

Merged
merged 5 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion build-fail-blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ recipes/unitig-counter
recipes/probcons
recipes/spydrpick
recipes/bctools
recipes/bayestyper
recipes/group_humann2_uniref_abundances_to_go
recipes/isonclust2
recipes/sibelia
Expand Down
45 changes: 45 additions & 0 deletions recipes/bayestyper/0001-CMakeLists.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 6bc8406..3a9a766 100755
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -42,9 +42,39 @@ else(${BUILD_STATIC} EQUAL 0)
endif(${BUILD_STATIC} EQUAL 0)


-FIND_PACKAGE(Boost COMPONENTS iostreams program_options system filesystem serialization REQUIRED)
+# When user specifies boost paths to use, we need to disable searching from the
+# environment and from using cmake's internal boost variables.
+if (BOOST_ROOT OR BOOST_INCLUDEDIR OR BOOST_LIBRARYDIR OR DEFINED ENV{BOOST_ROOT} OR DEFINED ENV{BOOST_INCLUDEDIR} OR DEFINED ENV{BOOST_LIBRARYDIR})
+ # Don't search for system include dirs for boost
+ set(Boost_NO_SYSTEM_PATHS ON)
+ # Disable excessive warning messages when using multiple boost versions
+ set(Boost_NO_WARN_NEW_VERSION ON)
+endif()
+
+# Use the OLD way of looking for Boost
+# The NEW way only respects BOOST_ROOT, not these more-specific variables.
+if (BOOST_INCLUDEDIR OR BOOST_LIBRARYDIR OR DEFINED ENV{BOOST_INCLUDEDIR} OR DEFINED ENV{BOOST_LIBRARYDIR})
+ # Ignore cmake boost variables
+ set(Boost_NO_BOOST_CMAKE ON)
+endif()
+
+set( Boost_USE_STATIC_LIBS OFF )
+set( Boost_USE_MULTITHREADED TRUE )
+set( Boost_DEBUG ON )
+
+find_package(Boost 1.71.0 COMPONENTS iostreams program_options system filesystem serialization REQUIRED)
message(STATUS ${Boost_LIBRARIES})

+if(Boost_FOUND)
+ include_directories(${Boost_INCLUDE_DIRS})
+ LINK_DIRECTORIES(${Boost_LIBRARY_DIRS})
+endif()
+
+MESSAGE("Boost configuration results:")
+MESSAGE(" Boost_INCLUDE_DIRS: ${Boost_INCLUDE_DIR}")
+MESSAGE(" Boost_LIBRARY_DIRS: ${Boost_LIBRARY_DIRS}")
+MESSAGE(" Boost_LIBRARIES: ${Boost_LIBRARIES}")
+
add_subdirectory(${CMAKE_SOURCE_DIR}/external/kmc_api)
add_subdirectory(${CMAKE_SOURCE_DIR}/src/vcf++)
add_subdirectory(${CMAKE_SOURCE_DIR}/src/kmerBloom)
28 changes: 28 additions & 0 deletions recipes/bayestyper/bayestyper.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
diff --git i/include/vcf++/VcfFile.hpp w/include/vcf++/VcfFile.hpp
index 4a2f69e..aaf49be 100644
--- i/include/vcf++/VcfFile.hpp
+++ w/include/vcf++/VcfFile.hpp
@@ -35,8 +35,8 @@ THE SOFTWARE.
#include <string>
#include <unordered_set>

-#include "boost/iostreams/filtering_stream.hpp"
-#include "boost/iostreams/filter/gzip.hpp"
+#include <boost/iostreams/filtering_stream.hpp>
+#include <boost/iostreams/filter/gzip.hpp>

#include "Variant.hpp"
#include "VcfMetaData.hpp"
diff --git i/src/vcf++/CMakeLists.txt w/src/vcf++/CMakeLists.txt
index c8a6142..beddd90 100755
--- i/src/vcf++/CMakeLists.txt
+++ w/src/vcf++/CMakeLists.txt
@@ -3,6 +3,8 @@ project(vcf++)
SET(LIBRARY_OUTPUT_PATH ${CMAKE_SOURCE_DIR}/lib)

include_directories(${CMAKE_SOURCE_DIR}/include/vcf++ ${Boost_INCLUDE_DIRS})
+message(STATUS "====== DEBUG Boost_INCLUDE_DIRS: " ${Boost_INCLUDE_DIRS})
+message(STATUS "====== DEBUG Boost_LIB_DIRS: " ${Boost_LIB_DIRS})
link_directories(${Boost_LIB_DIRS})

add_library(${PROJECT_NAME} VcfFile.cpp Variant.cpp Allele.cpp VcfMetaData.cpp AttributeSet.cpp Sample.cpp Utils.cpp Attribute.cpp JoiningString.cpp Stats.cpp AttributeFilter.cpp SampleAlleleAttributeFilter.cpp FastaReader.cpp FastaRecord.cpp Auxiliaries.cpp Trio.cpp Contig.cpp Regions.cpp)
30 changes: 24 additions & 6 deletions recipes/bayestyper/build.sh
Original file line number Diff line number Diff line change
@@ -1,10 +1,28 @@
#!/bin/bash

mkdir build && cd build
cmake ..
make

mkdir -p ${PREFIX}/bin

cp ../bin/bayesTyper ${PREFIX}/bin
cp ../bin/bayesTyperTools ${PREFIX}/bin
export INCLUDES="-I${PREFIX}/include"
export CPLUS_INCLUDE_PATH="${PREFIX}/include"
export LIBPATH="-L${PREFIX}/lib"
export LDFLAGS="${LDFLAGS} -L${PREFIX}/lib"
export CXXFLAGS="${CXXFLAGS} -O3 -std=c++14 -I${PREFIX}/include"
Comment on lines +5 to +9
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Make -O3 optimization configurable for arm64 builds

Currently, -O3 is hardcoded in CXXFLAGS for arm64/aarch64 builds in recipes/bayestyper/build.sh. Consider using -O2 for better stability or making the optimization level configurable based on the target architecture.

🔗 Analysis chain

Consider architecture-specific optimizations for arm64

The hardcoded -O3 optimization level might be too aggressive for arm64/aarch64 builds. Consider using -O2 for better stability or making it configurable based on the target architecture.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any existing arm64 builds using O3 optimization
rg -l 'CXXFLAGS.*-O3' recipes/*/build.sh | xargs rg 'host.*aarch64|arm64'

Length of output: 192942


if [[ `uname` == "Darwin" ]]; then
export CONFIG_ARGS="-DCMAKE_FIND_FRAMEWORK=NEVER -DCMAKE_FIND_APPBUNDLE=NEVER"
export CXXFLAGS="${CXXFLAGS} -Wno-dev -Wno-deprecated-declarations -Wno-unused-command-line-argument"
else
export CONFIG_ARGS=""
fi

cmake -S . -B build -DCMAKE_BUILD_TYPE=Release \
-DCMAKE_INSTALL_PREFIX="${PREFIX}" \
-DCMAKE_CXX_COMPILER="${CXX}" \
-DCMAKE_CXX_FLAGS="${CXXFLAGS}" \
-DBOOST_ROOT="${PREFIX}" \
"${CONFIG_ARGS}"
Comment on lines +18 to +23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unnecessary quotes around ${CONFIG_ARGS}

Enclosing ${CONFIG_ARGS} in quotes can lead to issues when the variable is empty or contains multiple arguments. This can cause CMake to interpret it incorrectly. Remove the quotes to allow proper argument expansion.

Apply this diff to fix the issue:

 cmake -S . -B build -DCMAKE_BUILD_TYPE=Release \
 	-DCMAKE_INSTALL_PREFIX="${PREFIX}" \
 	-DCMAKE_CXX_COMPILER="${CXX}" \
 	-DCMAKE_CXX_FLAGS="${CXXFLAGS}" \
 	-DBOOST_ROOT="${PREFIX}" \
-	"${CONFIG_ARGS}"
+	${CONFIG_ARGS}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cmake -S . -B build -DCMAKE_BUILD_TYPE=Release \
-DCMAKE_INSTALL_PREFIX="${PREFIX}" \
-DCMAKE_CXX_COMPILER="${CXX}" \
-DCMAKE_CXX_FLAGS="${CXXFLAGS}" \
-DBOOST_ROOT="${PREFIX}" \
"${CONFIG_ARGS}"
cmake -S . -B build -DCMAKE_BUILD_TYPE=Release \
-DCMAKE_INSTALL_PREFIX="${PREFIX}" \
-DCMAKE_CXX_COMPILER="${CXX}" \
-DCMAKE_CXX_FLAGS="${CXXFLAGS}" \
-DBOOST_ROOT="${PREFIX}" \
${CONFIG_ARGS}

cmake --build build -j "${CPU_COUNT}"

chmod 0755 bin/*
mv bin/bayesTyper ${PREFIX}/bin
mv bin/bayesTyperTools ${PREFIX}/bin
18 changes: 18 additions & 0 deletions recipes/bayestyper/kmer_defs.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
diff --git a/external/kmc_api/kmer_defs.h b/external/kmc_api/kmer_defs.h
index 8328822..89ae5e0 100644
--- a/external/kmc_api/kmer_defs.h
+++ b/external/kmc_api/kmer_defs.h
@@ -33,7 +33,12 @@


#include <stdio.h>
- #include <ext/algorithm>
+ #if defined(__clang__)
+ #include <algorithm>
+ #elif defined(__GNUC__)
+ #include <ext/algorithm>
+ using __gnu_cxx::copy_n;
+ #endif
#include <iostream>
using namespace std;

35 changes: 25 additions & 10 deletions recipes/bayestyper/meta.yaml
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
{% set name = "bayestyper" %}
{% set version = "1.5" %}

package:
name: bayestyper
name: {{ name }}
version: {{ version }}

build:
number: 1
skip: True # [osx]

source:
url: https://github.com/bioinformatics-centre/BayesTyper/archive/v{{ version }}.tar.gz
sha256: 917cd1b1ca7b5cfb6b8327138515ca1ad70878f0c4e8df393fddcbe42f281e14
patches:
- 0001-CMakeLists.patch
- bayestyper.patch
- kmer_defs.patch # [osx]
- stdafx.patch # [osx]

build:
number: 2
run_exports:
- {{ pin_subpackage('bayestyper', max_pin="x") }}

requirements:
build:
Expand All @@ -21,15 +28,23 @@ requirements:
- zlib
- boost-cpp
run:
- zlib
- boost-cpp

about:
home: https://github.com/bioinformatics-centre/BayesTyper
home: "https://github.com/bioinformatics-centre/BayesTyper"
license: MIT
summary: A method for variant graph genotyping based on exact alignment of k-mers
license_family: MIT
summary: "A method for variant graph genotyping based on exact alignment of k-mers."
dev_url: "https://github.com/bioinformatics-centre/BayesTyper"
doc_url: "https://github.com/bioinformatics-centre/BayesTyper/blob/v{{ version }}/README.md"

test:
commands:
- bayesTyper | grep BayesTyper
- bayesTyperTools | grep BayesTyperTools
- bayesTyper | grep "BayesTyper"
- bayesTyperTools | grep "BayesTyperTools"
identifiers:
- biotools:bayestyper
- doi:10.1038/s41588-018-0145-5
additional-platforms:
- linux-aarch64
- osx-arm64
10 changes: 10 additions & 0 deletions recipes/bayestyper/stdafx.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
diff --git a/external/kmc_api/stdafx.h b/external/kmc_api/stdafx.h
index 5b8d29d..3610d6c 100644
--- a/external/kmc_api/stdafx.h
+++ b/external/kmc_api/stdafx.h
@@ -1,4 +1,4 @@
#include <stdio.h>
-#include <ext/algorithm>
+#include <algorithm>
#include <iostream>
using namespace std;