Cross-platform: referenceless CRAM, macOS readFilesCommand fix, big-endian support#7
Merged
Merged
Conversation
The POSIX path used vfork()+execlp()+exit(0) to run the generated
readsCommand script. On macOS this fails ("Failed spawning
readFilesCommand", upstream issue alexdobin#2663) because the script has no
shebang and posix_spawn-style exec has no ENOEXEC->/bin/sh fallback;
calling exit() in a vfork child is also undefined behavior.
Replace with posix_spawnp invoking "<shell> <script>" explicitly, with
proper error checking. POSIX-only branch; the Windows system()-based
path is unchanged. Ported from upstream PR alexdobin#2693.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
STAR read the genome, suffix array and packed arrays through native uint*/uint128* casts, assuming little-endian byte order, which caused "next index is smaller than previous" failures on big-endian hosts (s390x, ppc64). Add byteOrder.h with loadUintLE/storeUintLE helpers and route the four endian-sensitive sites (PackedArray::operator[]/writePacked, funCompareSuffixes, funCalcSAiFromSA) through them. The helpers are guarded by STAR_BIG_ENDIAN: little-endian builds keep the original single-instruction native load (zero performance/behavior change), and only known big-endian compiles take the portable byte-wise path. The existing MSVC byte-access path in funCalcSAiFromSA is reused for BE. The byte-wise implementation is exposed by name and unit-tested (test_byteOrder.cpp) so the BE algorithm is validated on the LE CI runner. Ported from the patch in upstream issue alexdobin#2690. Note: compile-validated only; CI has no big-endian runner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add CRAM as an output container. STAR produces its BAM via the proven output path, then transcodes it to referenceless CRAM at finalization (cramOutput.cpp, using bundled HTSlib with CRAM_OPT_NO_REF, so no external reference FASTA is required). On conversion failure the original BAM is kept and the run continues. - Parameters_runtimeSetup: accept "CRAM" as outSAMtype[0], reusing the BAM Unsorted/SortedByCoordinate logic; set outCRAMbool and CRAM names. - STAR.cpp: transcode Aligned.out.bam / Aligned.sortedByCoord.out.bam to .cram after wiggle output, removing the intermediate BAM. - Hot per-record output code is untouched; the transcriptome BAM stays BAM for RSEM/Salmon. CRAM is incompatible with --outStd. - WebUI: CRAM options in the outSAMtype dropdown, .cram artifacts and MIME type. - Registered cramOutput in CMake build. Implemented properly rather than porting the incomplete upstream draft PR alexdobin#2670 (which wrote a NULL header and rerouted the working BAM path). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Port the platform-detection from upstream PR alexdobin#2694 to the Makefile (CI uses CMake, which already handles ARM64; this is for `make` users): - detect UNAME_S/UNAME_M; select -mavx2 on x86_64, -march=armv8-a+simd on ARM64/AArch64 - macOS + Apple clang OpenMP: -Xpreprocessor -fopenmp + libomp include, -lomp at link; add native STARforMac target (dynamic libomp) - portable BSD/macOS date fallback for the build timestamp - add cramOutput.o to OBJECTS Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements four requested cross-platform features. Each is a focused commit.
1. Referenceless CRAM output (
--outSAMtype CRAM Unsorted|SortedByCoordinate)STAR produces its normal BAM via the proven output path, then transcodes it to referenceless CRAM at finalization (
source/cramOutput.cpp, bundled HTSlibCRAM_OPT_NO_REF). No external reference FASTA required. On conversion failure the original BAM is kept and the run continues. Hot per-record output code is untouched; the transcriptome BAM (--quantMode TranscriptomeSAM) stays BAM for RSEM/Salmon. Selectable in the Web UI.Implemented properly rather than porting the incomplete upstream draft #2670 (which wrote a
NULLheader and rerouted the working BAM path). Referenceless was chosen over reference-based after analysis: on full-quality data reference compression only shrinks SEQ (quality scores dominate), so the size gain is ~10–20% for substantially more complexity/failure modes.2. macOS:
readFilesCommandviaposix_spawnpReplaces
vfork()+execlp()+exit(0)(POSIX branch) withposix_spawnpinvoking<shell> <script>, fixing "Failed spawning readFilesCommand" with gzipped input on macOS (#2663). Windowssystem()path unchanged. Ported from #2693.3. Big-endian support (guarded)
New
⚠️ Compile-validated only — CI has no big-endian runner.
source/byteOrder.h; the four endian-sensitive sites (PackedArray::operator[]/writePacked,funCompareSuffixes,funCalcSAiFromSA) read/write the little-endian byte stream explicitly. Guarded bySTAR_BIG_ENDIAN: little-endian keeps the native single-instruction load (zero perf/behavior change); only big-endian compiles take the portable path. The byte-wise impl is exposed and unit-tested (test_byteOrder.cpp) so the BE algorithm is validated on the LE CI runner. Ported from #2690.4. ARM64 / macOS build parity
CMake already auto-disables AVX2 on ARM64 (CI builds
macos-aarch64). This adds the same to the Makefile formakeusers (arch-detected SIMD, Apple-clang libomp, nativeSTARforMactarget, BSDdatefallback). Ported from #2694.Already present (verified, not re-done)
outputSJ.cpp).pubsetbuf/libc++ fix #2691 — already solved by the fork'sFixedStreamBuf.h.Testing
.cppsyntax-checked against MSVC STL (clang++ -std=c++17); both LE and forced-BE branches compile.test_byteOrder.cppunit tests added to the CTest suite.posix_spawnpath which cannot be tested on the Windows dev host.