From 3393ecda02cc93c635476320bf88aeed647d549d Mon Sep 17 00:00:00 2001 From: Glenn Hickey Date: Sun, 11 Oct 2026 21:18:39 -0400 Subject: [PATCH] Update abPOA to v1.5.7 with the score width fix, and adapt to its API Move abPOA from 017a3ac (v1.5.1, January 2024) to 12f99c4 on the CGT fork (ComparativeGenomicsToolkit/abPOA#14): upstream v1.5.7, the fork's packed DP rows, and a fix that picks the 16- or 32-bit score width from bounds on the scores, checks the results it cannot prove, and redoes a failed 16-bit alignment in 32 bits instead of exiting. This is the abPOA cactus is moving to. The submodule URL now points at the CGT fork, as cactus's does, since that is where the commit lives. 017a3ac also predates upstream's fix for DP cells past the band end (abPOA #66). With it, taffy norm reads uninitialised memory inside abPOA on taffy's own test data (valgrind), its output can change from one run to the next, and on a window that once failed with "Error in cg_backtrack" it gives an under-aligned MSA that depends on the SIMD build. The newer abPOA needs these changes here: - Since v1.5.6, abpoa_msa only starts afresh when the handle holds no sequences; otherwise it adds the new ones to the previous graph. Reset the shared handle before every call. Without this, taffy norm on the evolverMammals test grows past 9 GB and is killed. - With avx2=1, which include.mk exports, abPOA names its library libabpoa_avx2.a. Link whichever library was built as libabpoa.a, as cactus does. - v1.5.4 made "keep the last gap at the end", which used to be always on, an option that is off by default. Turn it back on (put_gap_at_end), so that gaps are placed as before. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV --- .gitmodules | 2 +- Makefile | 6 +++++- taffy/impl/merge_adjacent_alignments.c | 16 ++++++++++------ taffy/submodules/abPOA | 2 +- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/.gitmodules b/.gitmodules index bff9d10..20754d4 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,4 +4,4 @@ url = https://github.com/ComparativeGenomicsToolkit/sonLib [submodule "taffy/submodules/abPOA"] path = taffy/submodules/abPOA - url = https://github.com/yangao07/abPOA.git + url = https://github.com/ComparativeGenomicsToolkit/abPOA.git diff --git a/Makefile b/Makefile index e1432d1..92a8fd7 100644 --- a/Makefile +++ b/Makefile @@ -18,10 +18,14 @@ sonLib: ln -f ${sonLibDir}/*.a ${LIBDIR} ln -f ${sonLibDir}/sonLib.a ${LIBDIR}/libsonLib.a +# abPOA names its library after the SIMD level it is built for (avx2=1 gives libabpoa_avx2.a, no +# level gives libabpoa.a), so link whichever one was built as libabpoa.a, as cactus does abPOA: mkdir -p ${LIBDIR} ${INCLDIR} cd taffy/submodules/abPOA && ${MAKE} - ln -f taffy/submodules/abPOA/lib/*.a ${LIBDIR} + for lib in libabpoa_sse2.a libabpoa_sse41.a libabpoa_avx2.a libabpoa_avx512bw.a libabpoa.a; do \ + if [ -f taffy/submodules/abPOA/lib/$${lib} ]; then ln -f taffy/submodules/abPOA/lib/$${lib} ${LIBDIR}/libabpoa.a; fi; \ + done ln -f taffy/submodules/abPOA/include/*.h ${INCLDIR} rm -fr ${INCLDIR}/simde && cp -r taffy/submodules/abPOA/include/simde ${INCLDIR} diff --git a/taffy/impl/merge_adjacent_alignments.c b/taffy/impl/merge_adjacent_alignments.c index c6ea44c..229036a 100644 --- a/taffy/impl/merge_adjacent_alignments.c +++ b/taffy/impl/merge_adjacent_alignments.c @@ -272,6 +272,9 @@ static abpoa_para_t* construct_abpoa_params(void) { // progressive toggle abpt->progressive_poa = 1; + // keep the last gap at the end of the alignment, as abPOA always did before v1.5.4 + abpt->put_gap_at_end = 1; + // generate the substitution matrix abpt->use_score_matrix = 0; abpoa_post_set_para(abpt); @@ -310,9 +313,9 @@ static int len_rev_cmp(const void* i1, const void* i2, void* length_list) { * abPOA is set up once and reused for every gap alignment. Building the parameters is expensive out * of proportion to the work: abpoa_post_set_para refills a 65536 entry global lookup table each * time, and doing that once per merge dominated the run time of taffy norm on alignments made of - * many small blocks. The parameters are constant, and abpoa_msa resets the handle as its first act, - * so one instance serves every call. This is not thread safe, and neither is abPOA's use of the - * global tables that make it worth doing. + * many small blocks. The parameters are constant, and the handle is reset before every call (see + * align_interstitial_gaps_abpoa), so one instance serves every call. This is not thread safe, and + * neither is abPOA's use of the global tables that make it worth doing. */ static abpoa_t *shared_abpoa = NULL; static abpoa_para_t *shared_abpoa_params = NULL; @@ -396,10 +399,11 @@ int64_t align_interstitial_gaps_abpoa(Alignment *alignment) { } // run abpoa: todo try sorting on length - // abpoa_msa resets the shared handle as its first act, but it returns before doing so when - // handed no sequences, which would leave the previous call's alignment for us to read back. The - // seq_no == 0 return above means that cannot happen; this is here so it stays that way. + // abpoa_msa only starts afresh when the handle holds no sequences: since abPOA v1.5.6 it otherwise + // adds the new ones to the previous call's graph (incremental MSA). So reset the shared handle + // here, to the size abpoa_msa itself resets to. assert(seq_no > 0); + abpoa_reset(ab, abpoa_params, 1024); abpoa_msa(ab, abpoa_params, seq_no, NULL, seq_lens, bseqs, NULL, NULL); // copy the results from the abpoa matrix back into the left_gap_sequences diff --git a/taffy/submodules/abPOA b/taffy/submodules/abPOA index 017a3ac..12f99c4 160000 --- a/taffy/submodules/abPOA +++ b/taffy/submodules/abPOA @@ -1 +1 @@ -Subproject commit 017a3ac3a7084b5a574dc6f100bf7f4fb4ea44e1 +Subproject commit 12f99c4bc2a9f92546d583e111bd15245a21da23