From 15331df4508b1d85bba61422f38a5f294ba444c0 Mon Sep 17 00:00:00 2001 From: AlexanderMitrofanov Date: Wed, 14 Oct 2026 15:48:48 +0200 Subject: [PATCH 1/4] Fix ViennaRNA setup races and parallelize prepared accessibility folding --- AGENTS.md | 9 +- ChangeLog | 23 ++ README.md | 8 + doc/Makefile.am | 2 + .../vienna-accessibility-thread-safety.md | 293 +++++++++++++++++ doc/analysis/vrna-window-threads.cpp | 297 ++++++++++++++++++ src/IntaRNA/AccessibilityVrna.cpp | 33 +- src/IntaRNA/AccessibilityVrna.h | 6 + src/IntaRNA/InteractionEnergyVrna.cpp | 22 +- src/IntaRNA/VrnaHandler.cpp | 39 +++ src/IntaRNA/VrnaHandler.h | 21 +- src/bin/IntaRNA.cpp | 2 +- tests/AccessibilityVrna_test.cpp | 106 +++++++ tests/Makefile.am | 2 +- tests/runAccessibilityThreads.sh | 62 ++++ 15 files changed, 898 insertions(+), 27 deletions(-) create mode 100644 doc/analysis/vienna-accessibility-thread-safety.md create mode 100644 doc/analysis/vrna-window-threads.cpp create mode 100644 tests/runAccessibilityThreads.sh diff --git a/AGENTS.md b/AGENTS.md index 46333509..c39f3177 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -150,8 +150,13 @@ Example::isComplete() const comparison helpers from [general.h](src/IntaRNA/general.h). Internal energies use hundredths of kcal/mol; do not mix them with kcal/mol or Boltzmann values. - Preserve ownership and lifetimes when introducing RAII or non-owning views. - Retain synchronization around shared state and ViennaRNA calls; existing - accessibility calculations require serialization for thread safety. + Retain synchronization around shared ViennaRNA state: parameter/compound + setup, constraints and preparation use `VrnaHandler::getSetupMutex()`. + Prepared window accessibility computations can run concurrently with an + OpenMP-enabled ViennaRNA build; global MFE/PF calls remain serialized. + Complete parameter loading before workers start, preserve the ViennaRNA + logger lock, and consult `doc/analysis/vienna-accessibility-thread-safety.md` + before changing these boundaries. - For recurrence or predictor changes, extend the relevant regression tests and short-sequence oracles (`PredictorTinyOracle_test.cpp` and `PredictorSeedOracle_test.cpp`). For optimizations, compare correctness first, diff --git a/ChangeLog b/ChangeLog index de36b63d..18bc7d4b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -42,6 +42,10 @@ ## Technical changes and Optimizations +- fix ViennaRNA parameter-cache races and enable parallel prepared window + accessibility; retain serialization for global MFE/PF and non-OpenMP + ViennaRNA builds, with a source audit and concurrency regressions (issue #265) + - add an API overview, IntaRNA Doxygen styling, and automated documentation builds and GitHub Pages publishing alongside the user guide (issue #22) @@ -129,6 +133,25 @@ energies, restricted partition sums, and trackers. ################################################################################ ################################################################################ +261014 Alexander Mitrofanov + * IntaRNA/VrnaHandler, IntaRNA/AccessibilityVrna, + IntaRNA/InteractionEnergyVrna, bin/IntaRNA : + * serialize shared parameter/compound setup, constraints and preparation + through one exception-safe mutex, including native-thread library callers + * run prepared window probability recurrences concurrently with OpenMP + ViennaRNA; retain the conservative fallback for other builds + * protect global MFE/PF compatibility state, remove worker-side global model + resets, and install a separate logger lock for ViennaRNA >= 2.7 + * tests/AccessibilityVrna_test.cpp, tests/runAccessibilityThreads.sh, + tests/Makefile.am : + + compare serial/native-thread/OpenMP ED, ES and ensemble energies with + varied models; compare CLI accessibilities/predictions with SHAPE D/W + * doc/analysis/vienna-accessibility-thread-safety.md, + doc/analysis/vrna-window-threads.cpp, doc/Makefile.am, README.md, AGENTS.md : + + trace IntaRNA and ViennaRNA shared/owned state, provide an independent + race reproducer and document supported concurrency boundaries + * resolves https://github.com/BackofenLab/IntaRNA/issues/265 + 261007 Alexander Mitrofanov * doc/doxygen.cfg, doc/doxygen/* : + introduce the API main page with component links, prediction flow, diff --git a/README.md b/README.md index 8c12ed48..211d4943 100644 --- a/README.md +++ b/README.md @@ -457,6 +457,14 @@ The maximal number of threads to be used can be specified using the `--threads` If `--threads=k != 1`, than *k* predictions are processed in parallel. A value of `0` requests the maximally available number of threads for this machine. +ViennaRNA accessibility calculations for independent sequences run concurrently +when ViennaRNA was built with OpenMP support. IntaRNA serializes parameter and +constraint setup to protect ViennaRNA's shared caches; builds without ViennaRNA +OpenMP support retain serial accessibility calculation. Global intramolecular +MFE/partition-function calls remain serialized. See the +[thread-safety audit](doc/analysis/vienna-accessibility-thread-safety.md) for the +call sequence, source evidence, standalone reproducer and validation. + When using parallelization, you should have the following in mind: - The memory consumption will be multiplied by the number of threads, diff --git a/doc/Makefile.am b/doc/Makefile.am index cb78b081..6919154c 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -4,6 +4,8 @@ ################################################################ EXTRA_DIST = \ + analysis/vienna-accessibility-thread-safety.md \ + analysis/vrna-window-threads.cpp \ conda.txt \ api-documentation.md \ build-api.sh \ diff --git a/doc/analysis/vienna-accessibility-thread-safety.md b/doc/analysis/vienna-accessibility-thread-safety.md new file mode 100644 index 00000000..7f5668d0 --- /dev/null +++ b/doc/analysis/vienna-accessibility-thread-safety.md @@ -0,0 +1,293 @@ +# ViennaRNA accessibility thread-safety audit + +This audit accompanies [issue #265](https://github.com/BackofenLab/IntaRNA/issues/265). +The former critical section covered only `vrna_probs_window()`. That boundary +did not protect parameter construction: independent fold compounds can race +while ViennaRNA initializes them. Conversely, the sliding-window recurrence +uses compound-owned state and can run concurrently after synchronized setup. + +## Reviewed upstream sources + +The review covers the minimum supported ViennaRNA release, **2.4.14**, and the +**2.7.2** release used for the runtime experiments. Source links below are +pinned to immutable commits: + +| Release | Commit | Scope | +| --- | --- | --- | +| 2.4.14 | [`0b8ca4c6604e7a8bea366af62d82f8f40cea49bf`](https://github.com/ViennaRNA/ViennaRNA/tree/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf) | Minimum supported API and implementation | +| 2.7.2 | [`1ffec79f5e258896160f7362ced8263450f371dc`](https://github.com/ViennaRNA/ViennaRNA/tree/1ffec79f5e258896160f7362ced8263450f371dc) | Installed runtime and current parameter cache implementation | + +This is a review of the paths IntaRNA calls, not a blanket claim that RNAlib, +its deprecated interfaces, or every future release is thread-safe. In +particular, compiling RNAlib with OpenMP does not make all its static state +thread-private. + +## Call path and ownership + +The relevant accessibility path is: + +```text +AccessibilityVrna::fillByRNAplfold + VrnaHandler::getModel -> vrna_md_copy + vrna_fold_compound(PF | WINDOW) + add_params -> vrna_params + vrna_params_prepare -> vrna_exp_params + sanitize_bp_span, sequence/constraint/matrix setup + addConstraints + vrna_constraints_add -> vrna_hc_add_from_db + vrna_constraints_add_SHAPE, when requested + vrna_fold_compound_prepare(PF | WINDOW) + vrna_probs_window + vrna_fold_compound_prepare(PF | WINDOW) + allocate local auxiliary arrays + update compound-owned hard/soft constraints and DP matrices + callbackForStorage -> this accessibility object's ED matrix + vrna_fold_compound_free +``` + +The explicit preparation before `vrna_probs_window()` is significant. The +window routine calls `vrna_fold_compound_prepare()` internally, and parameter +preparation can replace `exp_params` when its model differs from `params`. +Construction sanitizes span/window settings after initially constructing +parameters. Thus the synchronization rule must cover preparation as well as +the constructor. See the +[2.7.2 compound constructor and preparation](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/fold_compound.c#L200-L276), +[preparation call chain](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/fold_compound.c#L519-L626), +and [parameter replacement condition](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/params/params.c#L427-L456). + +After explicit preparation, the second preparation is idempotent with respect +to parameter construction for this path: the two parameter models match, +constraints do not change them, and no other worker accesses this compound. +Hard/soft constraint preparation and matrix scaling operate on its owned +storage. This reasoning assumes IntaRNA's single-sequence model, with no +external callbacks or custom grammar that mutate model settings. See +[hard constraints](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/constraints/hard.c#L276-L310), +[soft constraints](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/constraints/soft.c#L304-L356), +and [matrix preparation](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/datastructures/dp_matrices.c#L299-L390). + +## Shared state and required synchronization + +| State or operation | Evidence | Consequence | +| --- | --- | --- | +| Parameter caches `p_pre`, `pf_pre`, and their initialization flags | [2.7.2 `params.c`](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/params/params.c#L88-L217) enables `SPEEDUP_PARAMS`; both constructors rewrite cached model members and copy whole cached structures. Only `id` and `pf_id` are declared OpenMP thread-private. | Guard every IntaRNA entry to parameter construction with one shared mutex. Separate locks for accessibility and energy construction would not protect the same caches. | +| Parameter identifiers in older RNAlib | [2.4.14 declarations](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/params/params.c#L88-L98), [identifier increment](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/params/params.c#L552) | 2.4.14 lacks the newer caches, but its identifier is shared in builds without OpenMP. The same setup lock remains useful. | +| Global model defaults and legacy model variables | [2.7.2 `vrna_md_defaults_reset`](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/model.c#L346-L432), [2.4.14 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/model.c#L311-L384) | Passing a model to `vrna_md_defaults_reset()` changes process-wide defaults; it does not merely reset that local object. Remove the worker-side call when every fold already receives its explicit model. Guard parameter-file loading/default reads during handler setup. | +| `vrna_md_copy` | [2.7.2 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/model.c#L196-L232) copies the supplied structure and its arrays. | Copying a stable handler model into worker-local storage needs no global lock. | +| Sliding-window DP, auxiliary arrays, and constraint updates | [2.7.2 window implementation](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/partfunc/pf_window.c#L681-L958), [2.4.14 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/LPfold.c#L669-L941) obtain arrays and models from the compound or allocate local helpers. | Independent prepared compounds can compute concurrently. Keep callback storage distinct and alive until each call returns. | +| Window compatibility state | [2.7.2 declarations](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/partfunc/pf_window.c#L91-L110), [legacy wrapper](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/partfunc/pf_window.c#L2170-L2243) | `backward_compat_compound` and `backward_compat` belong to the legacy wrapper path. The direct `vrna_probs_window()` call does not set them. Do not replace the direct API with a legacy wrapper under the same concurrency assumptions. | +| SHAPE D, Z, and W input | [2.7.2 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/probing/SHAPE.c#L332-L404), [2.4.14 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/constraints/SHAPE.c#L61-L120) | Setup loads input into the compound's soft constraints. W reads supplied pseudoenergies; this API does not perform stochastic optimization. Include constraint setup in the guarded preparation boundary. | +| Destruction | [2.7.2 `vrna_fold_compound_free`](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/fold_compound.c#L117-L197) frees compound-owned arrays, constraints, and auxiliary data. | Ordinary accessibility destruction does not mutate the parameter caches. Each compound must have one owner. | +| Global PF compatibility pointer `pr` | [2.7.2 `vrna_pf`](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/partfunc/partfunc.c#L208-L222), [2.4.14 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/part_func.c#L129-L142) still write the process-global legacy pointer. | The accessibility conclusion does not justify unlocking global PF calls. Serialize the energy-model global-fold paths with the same mutex. | +| Lazy MFE SIMD dispatch | [2.7.2 dispatcher](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/utils/higher_order_functions.c#L55-L128) replaces a shared function pointer on first use. | Keep global MFE under synchronization as well; this is a different path from the accessibility recurrence. | +| Diagnostics | [2.7.2 logger](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/utils/log.c#L489-L552) lazily allocates callback storage; its default lock depends on the library's pthread build configuration. | OpenMP support alone is not proof for diagnostic paths. Install a dedicated logger lock through the public callback API before concurrent calls. | + +## Boundary and limitations + +The synchronization boundary must use a process-wide mutex shared by +`VrnaHandler`, `AccessibilityVrna`, and `InteractionEnergyVrna`; an OpenMP +critical section limited to one call site cannot cover the shared setup +state. RAII locking permits normal C++ exception unwinding and works when +IntaRNA itself is built without OpenMP. + +The parallel boundary is **create, constrain, and prepare while +locked; compute the window recurrence with exclusive ownership of the +compound**. Keep global MFE/PF calls synchronized. RNAlib builds without +OpenMP use a conservative full-call fallback. Do not concurrently mutate +the handler model, reload global parameter tables through external RNAlib +calls, modify the same compound, or replace callback state while a call is +running. IntaRNA's mutex cannot coordinate direct RNAlib calls made by other +libraries in an embedding application. + +This guarantee concerns the ViennaRNA boundary. Native-thread library users +must separately provide thread-safe IntaRNA/easylogging++ diagnostics, or leave +verbose/performance logging disabled. The CLI's OpenMP logging synchronization +is not a general native-thread logging contract. + +For ViennaRNA 2.7 and later, handler initialization installs a process-lifetime +mutex callback once through `vrna_log_lock_set()`. This replaces the logger's +locking hook while preserving its output streams, levels, and message +callbacks. The logger mutex is distinct from the setup mutex because guarded +ViennaRNA operations can emit messages themselves. This protects diagnostics +even if RNAlib was built without pthread support. Embedding applications must +not reset or replace that hook while IntaRNA folds are running; callbacks must +not reenter folding. Earlier reviewed logging uses stdio directly rather than +the newer lazily initialized callback array. See the +[public locking callback](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/utils/log.h#L61-L70) +and [hook setter](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/utils/log.c#L284-L290). + +The 2.4.14 source review establishes API availability and identifies its +state, but does not substitute for testing that release. Runtime comparisons +on 2.7.2 do not prove every intermediate release or build configuration safe. +Repeat the audit when changing the supported RNAlib range, adding model +mutations after preparation, changing SHAPE integration, or adding callbacks. + +## Runtime evidence and reproduction + +The diagnostic compares every unpaired probability returned by the callback +against a serial baseline, using independent compounds and callback storage. +It distinguishes fully concurrent lifecycles, serial creation followed by +parallel window calls, and synchronized preparation followed by parallel +window calls. Varying models matters: a shared cache can appear harmless when +all workers request identical parameters. + +The committed [standalone diagnostic](vrna-window-threads.cpp) uses 24 +deterministic sequences of varied lengths, windows, spans, and hard/soft +constraints. Its default profile also varies temperature and model flags. +`shared-model` fixes temperature to 37 degrees Celsius and uses the same +`noLP`/`noGUclosure` settings across cases, demonstrating that varying window +dimensions alone is sufficient to expose the cache race. + +Build with a C++23 compiler, OpenMP, and RNAlib's pkg-config metadata available: + +```bash +c++ -std=c++23 -O2 -fopenmp doc/analysis/vrna-window-threads.cpp \ + -o /tmp/vrna-window-threads $(pkg-config --cflags --libs RNAlib2) +/tmp/vrna-window-threads setup-locked 8 12 +/tmp/vrna-window-threads setup-locked 8 30 shared-model +/tmp/vrna-window-threads window-only 8 12 +``` + +The following modes intentionally exercise unsynchronized RNAlib state. Run +them as separate diagnostic processes, never as ordinary CI pass conditions: + +```bash +/tmp/vrna-window-threads setup-only 8 30 shared-model +/tmp/vrna-window-threads window-locked 8 12 +/tmp/vrna-window-threads unsafe 8 30 shared-model +``` + +Observed results used ViennaRNA 2.7.2's static library with OpenMP enabled, +GCC 14.4.0 (conda-forge build 14.4.0-7), and Linux x86-64 with CPU affinity +0-11. Runs requested eight workers; other build activity could share these +CPUs. No ViennaRNA 2.4.14 runtime was installed for the experiment. + +| Mode/profile | Completed repetitions | Exact probability comparisons | Result | +| --- | ---: | ---: | --- | +| `setup-locked`, mixed model | 12 | 4,046,016 | No parameter, probability, or callback metadata mismatch | +| `setup-locked`, shared model | 30 | 10,115,040 | No parameter, probability, or callback metadata mismatch | +| `window-only`, mixed model | 12 | 4,046,016 | No parameter, probability, or callback metadata mismatch | +| `setup-only`, shared model | 1 | Not applicable | 17 compounds with mismatched parameter snapshots | +| `window-locked`, mixed model | 2 | 674,336 | 6,120 probability mismatches, one parameter mismatch, one metadata mismatch | +| `unsafe`, shared model | 1 | 337,168 | 6,120 probability mismatches, one parameter mismatch, one metadata mismatch | + +A one-worker `unsafe` control completed three repetitions with 1,011,504 +exact comparisons and no mismatches. Five fresh `setup-only` processes also +each reproduced parameter corruption. The failing modes stop at the first +iteration containing a mismatch, so repetition counts and mismatch counts +will vary with scheduling. These experiments locate a concrete failure +outside the original recurrence-only lock; finite passing stress runs are +not a race-detector proof or a performance measurement. + +## Integrated regressions and existing SHAPE limitations + +GCC 14 release and `--enable-debug` builds each passed `make tests -j4`: +all seven test programs/scripts passed, including 75,901 API assertions in +69 test cases. The source base was `c5f1d51` on `master` (the repository has no +`main` branch). `git diff --check` and shell syntax validation also passed. + +`tests/AccessibilityVrna_test.cpp` compares all retained ED values (including +the extra band used for dangling ends), ES matrices, initiation energies and +both global ensemble energies against serial references. Twelve input/model +combinations vary sequence length, temperature, window, span, scaling and +hard constraints. Four native threads and four OpenMP workers each perform +48 jobs; assertions and exception reporting happen after workers join. + +`tests/runAccessibilityThreads.sh` compares sorted predictions and every +per-sequence accessibility export at one and two workers, with SHAPE D and W +as well as unconstrained input. Unconstrained and D cases also request Eall +output, exercising overlapping target accessibility and global folding. + +Two pre-existing serial failures in the installed ViennaRNA 2.7.2 path limit +SHAPE coverage. Z input crashes even without Eall output; W input crashes +when reported interactions request Eall. Both occur with the unmodified base +binary and the corrected binary at one thread. They are not introduced by +parallel window calculation, and are outside this synchronization change. +In particular, the existing `addConstraints()` passes window-specific options +also when called for global Eall, which is inappropriate for W's +`vrna_sc_set_up()` path. The regressions therefore exercise W without Eall and +do not claim working Z support. + +A minimal serial Z reproduction, using either binary, is: + +```bash +for i in $(seq 1 16); do printf '%s 0.5\n' "$i"; done > /tmp/reactivities.shape +IntaRNA --query=GGGGAAAACCCCUAGC --target=GGGGAAAACCCCUAGC --seedBP=3 \ + --qIntLenMax=8 --tIntLenMax=8 --qAccW=14 --tAccW=14 --qAccL=10 --tAccL=10 \ + --qShape=/tmp/reactivities.shape --tShape=/tmp/reactivities.shape \ + --qShapeMethod=Z --tShapeMethod=Z --outMode=C --outCsvCols=id1,id2,E,ED1,ED2 \ + --outMaxE=100 --threads=1 --default-log-file=/dev/null +``` + +An additional branch probe compiled the three changed implementation units +with `INTARNA_MULITHREADING=0` and without `-fopenmp`. Four native workers +matched serial ED, ES and Eall values for 48 jobs. A linker wrapper observed +four simultaneous `vrna_probs_window()` calls with the normal header and one +with an isolated header override removing `VRNA_WITH_OPENMP`. This verifies +both the parallel and serialized branches. It is **not** a runtime test of a +non-OpenMP RNAlib: the linked library remained the installed OpenMP-enabled +2.7.2, and unchanged IntaRNA objects came from the release archive. + +## End-to-end measurement + +The base and corrected binaries used the same GCC 14.4 release toolchain, +ViennaRNA 2.7.2 and options. The workload has 32 deterministic 400-nt queries, +a 6-nt target, and 120-nt accessibility windows. Kinetic interaction prediction +keeps this workload focused on accessibility. Every run exports all query ED +values as well as interaction structures, coordinates and energies. + +On a Ryzen 5 7530U, affinity CPUs 8-11 exposes four logical CPUs on two physical +cores. One warmup preceded three alternating measured runs per configuration. +Other compilation activity ran on CPUs 0-7 during this first measurement, so +these numbers are workload observations rather than general speedup claims. + +| Workers | Binary | Median wall time (range), s | Median peak RSS, KiB | +| ---: | --- | ---: | ---: | +| 1 | Base | 4.15 (4.11-5.07) | 18,376 | +| 1 | Corrected | 4.76 (4.51-5.92) | 18,392 | +| 4 | Base | 6.24 (4.71-6.48) | 30,396 | +| 4 | Corrected | 2.01 (1.98-2.03) | 30,564 | + +The four-worker median improved by 3.10x with comparable peak memory. To +check the noisy single-worker result, four further alternating runs pinned +to CPU 8 after compilation finished measured 3.865 s (3.77-4.09) for the base +and 3.890 s (3.84-3.95) for the correction. That 0.025-s difference lies within +the observed run variation; it does not establish zero overhead. + +All 24 runs matched the **base serial** reference exactly: sorted CSV output +(32 interaction rows) and all 32 ED exports (114,048 values per run). The +unmodified multiworker binary also happened to match on these inputs; its +passing benchmark does not negate the separate race reproducer. + +Generate the inputs with Python 3: + +```bash +mkdir -p /tmp/intarna-265-benchmark +python3 - <<'PY' +from pathlib import Path +from random import Random +out = Path('/tmp/intarna-265-benchmark') +rng = Random(265) +out.joinpath('queries.fa').write_text(''.join( + '>query%02d\n%s\n' % (i + 1, ''.join(rng.choice('ACGU') for _ in range(400))) + for i in range(32))) +out.joinpath('target.fa').write_text('>target\nGCGCGC\n') +PY +``` + +Use the following command with each binary, worker count, and a fresh output +directory for each repetition. Set `binary`, `threads`, and `out_dir` first; +adjust the CPU affinity to the local machine. The serial followup uses `8` +instead of `8-11`. + +```bash +mkdir -p "$out_dir" +export OMP_DYNAMIC=FALSE OMP_PROC_BIND=TRUE OMP_PLACES=cores OPENBLAS_NUM_THREADS=1 +/usr/bin/time -f '%e %U %S %M' -o "$out_dir/time.txt" \ + taskset -c 8-11 "$binary" \ + --query=/tmp/intarna-265-benchmark/queries.fa \ + --target=/tmp/intarna-265-benchmark/target.fa \ + --model=X --mode=K --seedBP=3 --qIntLenMax=8 --tIntLenMax=6 \ + --qAccW=120 --qAccL=120 --tAccW=6 --tAccL=6 --threads="$threads" \ + --outMode=C --outCsvCols=id1,start1,end1,id2,start2,end2,subseqDP,hybridDP,E,ED1,ED2 \ + --out="$out_dir/predictions.csv" --out="qAcc:$out_dir/query.ed" +``` diff --git a/doc/analysis/vrna-window-threads.cpp b/doc/analysis/vrna-window-threads.cpp new file mode 100644 index 00000000..3fa84780 --- /dev/null +++ b/doc/analysis/vrna-window-threads.cpp @@ -0,0 +1,297 @@ +/** + * Standalone ViennaRNA diagnostic for IntaRNA issue #265. + * + * Usage: vrna-window-threads MODE THREADS REPETITIONS [shared-model] + * MODE is setup-locked, window-only, unsafe, window-locked, or setup-only. + * The unsafe modes intentionally access ViennaRNA's shared parameter caches + * concurrently. Run them as separate diagnostic processes, never in normal CI. + * Passing this finite stress test does not establish thread safety. + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +extern "C" { +#include +#include +#include +#include +#include +#include +#include +#include +} + +namespace { + +constexpr unsigned int options = VRNA_OPTION_PF | VRNA_OPTION_WINDOW; +using Compound = std::unique_ptr; + +struct Input { + std::string sequence; + std::string constraint; + vrna_md_t model; + int window; + bool softConstraint; +}; + +struct Result { + int length; + int window; + int status = 0; + int callbacks = 0; + bool invalid = false; + std::vector parameters; + std::vector probabilities; + std::vector seen; + + Result(int length, int window) + : length(length), window(window), + probabilities((length + 1) * (window + 1)), seen(probabilities.size()) + {} +}; + +std::vector +makeInputs(bool sharedModel) +{ + std::vector inputs; + uint32_t rng = 0x265; + for (int id = 0; id < 24; ++id) { + Input input; + const int length = 77 + id * 13; + for (int i = 0; i < length; ++i) { + rng ^= rng << 13; + rng ^= rng >> 17; + rng ^= rng << 5; + input.sequence += "ACGU"[rng % 4]; + } + if (id % 4 == 0) { + input.sequence.replace(10, 20, "GGGGGGGAAAAAACCCCCCC"); + } + input.window = std::min(length, 32 + (id % 7) * 16); + vrna_md_set_default(&input.model); + input.model.window_size = input.window; + input.model.max_bp_span = id % 3 == 0 ? input.window - 8 : input.window; + input.model.temperature = sharedModel ? 37 : 20 + (id % 5) * 8.5; + input.model.noLP = sharedModel ? 0 : id % 2; + input.model.noGUclosure = sharedModel ? 0 : (id / 2) % 2; + input.model.dangles = 2; + input.model.gquad = 0; + input.model.compute_bpp = 1; + if (id % 3 == 1) { + input.constraint.assign(input.sequence.size(), '.'); + input.constraint.front() = input.constraint.back() = 'x'; + } + input.softConstraint = id % 3 == 2; + inputs.push_back(input); + } + return inputs; +} + +Compound +makeCompound(const Input &input) +{ + vrna_md_t model = input.model; + Compound compound(vrna_fold_compound(input.sequence.c_str(), &model, options), + &vrna_fold_compound_free); + if (!compound) { + std::cerr << "Fold compound allocation failed\n"; + std::exit(3); + } + if (!input.constraint.empty()) { + vrna_constraints_add(compound.get(), input.constraint.c_str(), + VRNA_CONSTRAINT_DB_DEFAULT | VRNA_CONSTRAINT_DB_ENFORCE_BP); + } + if (input.softConstraint) { + // Same per-position pseudoenergy storage used by unpaired SHAPE constraints. + vrna_sc_add_up(compound.get(), 10, 0.5, options); + vrna_sc_add_up(compound.get(), 20, -0.25, options); + } + // probs_window prepares again. Do the potentially cache-writing preparation + // here so that setup-locked includes model sanitization and parameter updates. + if (!vrna_fold_compound_prepare(compound.get(), options)) { + std::cerr << "Fold compound preparation failed\n"; + std::exit(3); + } + return compound; +} + +std::vector +parameterSnapshot(const vrna_fold_compound_t &compound) +{ + std::vector values; + for (const vrna_md_t *model : { &compound.params->model_details, + &compound.exp_params->model_details }) { + values.insert(values.end(), { model->temperature, + static_cast(model->window_size), static_cast(model->max_bp_span), + static_cast(model->min_loop_size), static_cast(model->noLP), + static_cast(model->noGUclosure), static_cast(model->dangles) }); + } + // Stable fields only: parameter IDs and structure padding are deliberately + // excluded; they need not be identical between independently created objects. + values.push_back(compound.exp_params->kT); + for (int i = 0; i <= NBPAIRS; ++i) { + for (int j = 0; j <= NBPAIRS; ++j) { + values.push_back(compound.params->stack[i][j]); + values.push_back(compound.exp_params->expstack[i][j]); + } + } + return values; +} + +void +capture(FLT_OR_DBL *probabilities, int size, int j, int max, + unsigned int type, void *data) +{ + auto &result = *static_cast(data); + if (!(type & VRNA_PROBS_WINDOW_UP)) { + return; + } + ++result.callbacks; + if (j < 1 || j > result.length) { + result.invalid = true; + return; + } + for (int length = 1; length <= std::min({ j, size, max, result.window }); ++length) { + const size_t index = j * (result.window + 1) + length; + const double probability = probabilities[length]; + result.probabilities[index] = probability; + result.seen[index] = 1; + if (!std::isfinite(probability) || probability < 0 || probability > 1.00000001) { + result.invalid = true; + } + } +} + +void +calculate(vrna_fold_compound_t *compound, const Input &input, Result &result) +{ + result.status = vrna_probs_window(compound, input.window, + VRNA_PROBS_WINDOW_UP, capture, &result); +} + +} // namespace + +int +main(int argc, char **argv) +{ + if (argc < 4 || argc > 5) { + std::cerr << "Usage: " << argv[0] + << " MODE THREADS REPETITIONS [shared-model]\n" + << "MODE: setup-locked | window-only | unsafe | window-locked | setup-only\n"; + return 2; + } + const std::string mode = argv[1]; + const int threads = std::atoi(argv[2]); + const int repetitions = std::atoi(argv[3]); + const bool sharedModel = argc == 5 && std::string(argv[4]) == "shared-model"; + if (threads < 1 || repetitions < 1 || + (argc == 5 && !sharedModel) || + (mode != "setup-locked" && mode != "window-only" && mode != "unsafe" && + mode != "window-locked" && mode != "setup-only")) { + std::cerr << "Invalid arguments\n"; + return 2; + } + omp_set_dynamic(0); + omp_set_num_threads(threads); + vrna_params_load_RNA_Turner2004(); + const auto inputs = makeInputs(sharedModel); + std::vector reference; + for (const auto &input : inputs) { + reference.emplace_back(input.sequence.size(), input.window); + auto compound = makeCompound(input); + reference.back().parameters = parameterSnapshot(*compound); + if (mode != "setup-only") { + calculate(compound.get(), input, reference.back()); + if (!reference.back().status || reference.back().invalid) { + std::cerr << "Invalid serial baseline\n"; + return 3; + } + } + } + + size_t compared = 0, probabilityMismatches = 0, parameterMismatches = 0, metadataMismatches = 0; + const auto start = std::chrono::steady_clock::now(); + int completed = 0; + for (int repetition = 0; repetition < repetitions; ++repetition) { + std::vector observed; + std::vector compounds; + for (const auto &input : inputs) { + observed.emplace_back(input.sequence.size(), input.window); + compounds.emplace_back(mode == "window-only" ? makeCompound(input).release() : nullptr, + &vrna_fold_compound_free); + } + #pragma omp parallel for schedule(dynamic, 1) + for (int id = 0; id < static_cast(inputs.size()); ++id) { + if (mode == "setup-locked") { + #pragma omp critical(vrna_probe_setup) + compounds[id] = makeCompound(inputs[id]); + } else if (mode != "window-only") { + compounds[id] = makeCompound(inputs[id]); + } + observed[id].parameters = parameterSnapshot(*compounds[id]); + if (mode == "window-locked") { + #pragma omp critical(vrna_probe_window) + calculate(compounds[id].get(), inputs[id], observed[id]); + } else if (mode != "setup-only") { + calculate(compounds[id].get(), inputs[id], observed[id]); + } + compounds[id].reset(); + } + ++completed; + for (size_t id = 0; id < inputs.size(); ++id) { + const auto &expected = reference[id]; + const auto &actual = observed[id]; + if (expected.parameters != actual.parameters) { + if (parameterMismatches < 10) { + const auto first = std::mismatch(expected.parameters.begin(), expected.parameters.end(), + actual.parameters.begin()); + std::cerr << std::setprecision(17) << "Parameter mismatch: repetition=" << repetition + << " case=" << id << " field=" << first.first - expected.parameters.begin() + << " serial=" << *first.first << " parallel=" << *first.second << '\n'; + } + ++parameterMismatches; + } + if (expected.status != actual.status || actual.invalid || + expected.callbacks != actual.callbacks || expected.seen != actual.seen) { + std::cerr << "Callback metadata mismatch: repetition=" << repetition << " case=" << id << '\n'; + ++metadataMismatches; + } + for (size_t index = 0; index < expected.probabilities.size(); ++index) { + if (!expected.seen[index]) { + continue; + } + ++compared; + if (expected.probabilities[index] != actual.probabilities[index]) { + if (probabilityMismatches < 10) { + std::cerr << std::setprecision(17) << "Probability mismatch: repetition=" << repetition + << " case=" << id << " end=" << index / (expected.window + 1) + << " length=" << index % (expected.window + 1) + << " serial=" << expected.probabilities[index] + << " parallel=" << actual.probabilities[index] << '\n'; + } + ++probabilityMismatches; + } + } + } + if (parameterMismatches || probabilityMismatches || metadataMismatches) { + break; + } + } + const double elapsed = std::chrono::duration(std::chrono::steady_clock::now() - start).count(); + std::cout << "ViennaRNA=" << VRNA_VERSION << " mode=" << mode << " threads=" << threads + << " repetitions=" << completed << '/' << repetitions << " cases=" << inputs.size() + << " profile=" << (sharedModel ? "shared-model" : "mixed-model") + << " compared=" << compared << " parameter_mismatches=" << parameterMismatches + << " probability_mismatches=" << probabilityMismatches + << " metadata_mismatches=" << metadataMismatches << " elapsed_seconds=" << elapsed << '\n'; + return parameterMismatches || probabilityMismatches || metadataMismatches ? 1 : 0; +} diff --git a/src/IntaRNA/AccessibilityVrna.cpp b/src/IntaRNA/AccessibilityVrna.cpp index ea88794f..8d53c829 100644 --- a/src/IntaRNA/AccessibilityVrna.cpp +++ b/src/IntaRNA/AccessibilityVrna.cpp @@ -28,6 +28,7 @@ // RNAplfold-like ED filling extern "C" { + #include #include #include #include @@ -222,7 +223,9 @@ fillByRNAplfold( const VrnaHandler &vrnaHandler // RnaSequence owns stable, null-terminated storage for the complete call. const std::string & sequence = getSequence().asString(); - // setup folding data + // Parameter creation uses shared ViennaRNA caches, even in OpenMP builds. + // Complete preparation under the same lock as the global MFE/PF callers. + std::unique_lock setupLock(VrnaHandler::getSetupMutex()); VrnaFoldCompoundPtr foldCompoundOwner( vrna_fold_compound( sequence.c_str(), &curModel, VRNA_OPTION_PF | VRNA_OPTION_WINDOW ) ); vrna_fold_compound_t * const fold_compound = foldCompoundOwner.get(); @@ -233,21 +236,27 @@ fillByRNAplfold( const VrnaHandler &vrnaHandler // add accessibility constraints addConstraints( *fold_compound, *this ); + // vrna_probs_window() prepares again internally. With unchanged model + // details, that second preparation only accesses this compound's own data. + if (!vrna_fold_compound_prepare(fold_compound, VRNA_OPTION_PF | VRNA_OPTION_WINDOW)) { + throw std::runtime_error("AccessibilityVrna::fillByRNAplfold() : vrna_fold_compound_prepare() failed"); + } + +#ifdef VRNA_WITH_OPENMP + // The expensive window recurrence and callback use per-compound storage. + // Retain full serialization for ViennaRNA builds without OpenMP support. + setupLock.unlock(); +#endif + // provide access to this object to be filled by the callback // and the normalized temperature for the Boltzmann weight computation std::pair< AccessibilityVrna*, FLT_OR_DBL > storageRT(this, (FLT_OR_DBL)vrnaHandler.getRT()); - // VRNA is not fully thread-safe, so we need to lock the call to vrna_probs_window() to avoid race conditions -#if INTARNA_MULITHREADING - #pragma omp critical(intarna_omp_vrna_probs_window) -# endif - { - // call folding and unpaired prob calculation - int retVal = vrna_probs_window( fold_compound, plFoldW, VRNA_PROBS_WINDOW_UP, &callbackForStorage, (void*)(&storageRT)); - // check if computations went fine - if (retVal == 0) { - throw std::runtime_error("AccessibilityVrna::fillByRNAplfold() : vrna_probs_window() returned 0 status ..."); - } + // call folding and unpaired prob calculation + int retVal = vrna_probs_window( fold_compound, plFoldW, VRNA_PROBS_WINDOW_UP, &callbackForStorage, (void*)(&storageRT)); + // check if computations went fine + if (retVal == 0) { + throw std::runtime_error("AccessibilityVrna::fillByRNAplfold() : vrna_probs_window() returned 0 status ..."); } } diff --git a/src/IntaRNA/AccessibilityVrna.h b/src/IntaRNA/AccessibilityVrna.h index d828ec3e..83a03c07 100644 --- a/src/IntaRNA/AccessibilityVrna.h +++ b/src/IntaRNA/AccessibilityVrna.h @@ -22,6 +22,10 @@ namespace IntaRNA { * Computes accessibility energy values for _all_ _regions_ using the free * energies of structure ensembles based on partition function computations * via the Vienna RNA package. + * ViennaRNA operations on independent objects are synchronized once global + * energy parameter loading is complete. Window folding runs in parallel when + * ViennaRNA was built with OpenMP support; otherwise it is serialized. Native + * thread callers must also configure thread-safe IntaRNA diagnostic logging. * * @author Martin Mann 2014 */ @@ -74,6 +78,8 @@ class AccessibilityVrna : public Accessibility { /** * Adds constraints to the VRNA fold compound that correspond to the present * AccessibilityConstraints + * The caller must hold VrnaHandler::getSetupMutex() during concurrent use of + * ViennaRNA, including preparation of SHAPE soft constraints. * @param fold_compound INOUT the object to extend * @param acc the accessibility handler to get constraint information from */ diff --git a/src/IntaRNA/InteractionEnergyVrna.cpp b/src/IntaRNA/InteractionEnergyVrna.cpp index 553226eb..07d5c277 100644 --- a/src/IntaRNA/InteractionEnergyVrna.cpp +++ b/src/IntaRNA/InteractionEnergyVrna.cpp @@ -39,6 +39,13 @@ struct VrnaParamDeleter { typedef std::unique_ptr VrnaFoldCompoundPtr; typedef std::unique_ptr VrnaParamPtr; +vrna_param_t * +getVrnaParams( vrna_md_t & model ) +{ + std::lock_guard setupLock(IntaRNA::VrnaHandler::getSetupMutex()); + return vrna_params(&model); +} + std::string getVrnaConstraint( const IntaRNA::Accessibility & acc ) { @@ -79,7 +86,7 @@ InteractionEnergyVrna::InteractionEnergyVrna( InteractionEnergy(accS1, accS2, maxInternalLoopSize1, maxInternalLoopSize2, energyAdd, energyWithDangles, internalLoopGU) // get final VRNA folding parameters , foldModel( vrnaHandler.getModel() ) - , foldParams( vrna_params( &foldModel ) ) + , foldParams( getVrnaParams( foldModel ) ) , RT(vrnaHandler.getRT()) , bpCG( BP_pair[RnaSequence::getCodeForChar('C')][RnaSequence::getCodeForChar('G')] ) , bpGC( BP_pair[RnaSequence::getCodeForChar('G')][RnaSequence::getCodeForChar('C')] ) @@ -92,15 +99,8 @@ InteractionEnergyVrna::InteractionEnergyVrna( // class destructor would otherwise not run on an exception. VrnaParamPtr foldParamsOwner(foldParams); - vrna_md_defaults_reset( &foldModel ); - // init ES values if needed if (initES) { -// 23.11.2017 : should not be relevant anymore -//#if INTARNA_MULITHREADING -// #pragma omp critical(intarna_omp_callingVRNA) -//#endif -// { // Compute into local owners and publish both matrices atomically only // after both ViennaRNA calls have completed successfully. std::unique_ptr esValues1Owner = std::make_unique(); @@ -110,7 +110,6 @@ InteractionEnergyVrna::InteractionEnergyVrna( computeES( accS2, *esValues2Owner ); esValues1 = esValues1Owner.release(); esValues2 = esValues2Owner.release(); -// } // omp critical(intarna_omp_callingVRNA) } // The fully constructed object resumes ownership through its unchanged raw @@ -139,6 +138,9 @@ void InteractionEnergyVrna:: computeES( const Accessibility & acc, InteractionEnergyVrna::EsMatrix & esToFill ) { + // Global MFE/PF routines also write ViennaRNA compatibility state and + // initialize shared dispatchers, so protect their full compound lifecycle. + std::lock_guard setupLock(VrnaHandler::getSetupMutex()); // prepare container esToFill.resize( acc.getSequence().size(), acc.getSequence().size() ); @@ -216,6 +218,8 @@ E_type InteractionEnergyVrna:: computeIntraEall( const Accessibility & acc ) const { + // Unlike prepared window folding, global PF still writes shared state. + std::lock_guard setupLock(VrnaHandler::getSetupMutex()); vrna_md_t curModel = foldModel; // avoid computation of base pair probabilities diff --git a/src/IntaRNA/VrnaHandler.cpp b/src/IntaRNA/VrnaHandler.cpp index fe3d196b..594af3c6 100644 --- a/src/IntaRNA/VrnaHandler.cpp +++ b/src/IntaRNA/VrnaHandler.cpp @@ -16,6 +16,28 @@ extern "C" { #include #include #include +#if VRNA_VERSION_MAJOR > 2 || (VRNA_VERSION_MAJOR == 2 && VRNA_VERSION_MINOR >= 7) + #include +#endif +} + +namespace { + +#if VRNA_VERSION_MAJOR > 2 || (VRNA_VERSION_MAJOR == 2 && VRNA_VERSION_MINOR >= 7) +void +lockVrnaLogging( const int lock, void * ) noexcept +{ + // Logging can occur while the setup mutex is held. Use a separate mutex + // for ViennaRNA builds whose logger has no built-in pthread protection. + static std::mutex loggingMutex; + if (lock) { + loggingMutex.lock(); + } else { + loggingMutex.unlock(); + } +} +#endif + } namespace IntaRNA { @@ -31,6 +53,13 @@ VrnaHandler( Z_type temperature model() , RT(getRT(temperature)) { + std::lock_guard setupLock(getSetupMutex()); + +#if VRNA_VERSION_MAJOR > 2 || (VRNA_VERSION_MAJOR == 2 && VRNA_VERSION_MINOR >= 7) + // Install only the locking callback; preserve log outputs and user callbacks. + static std::once_flag loggingInitialized; + std::call_once(loggingInitialized, [] { vrna_log_lock_set(lockVrnaLogging, nullptr); }); +#endif // load parameters if ( !vrnaParamFile.empty() ) { @@ -142,6 +171,16 @@ getPfScaleDefault() //////////////////////////////////////////////////////////////////////////// +std::mutex & +VrnaHandler:: +getSetupMutex() +{ + static std::mutex setupMutex; + return setupMutex; +} + +//////////////////////////////////////////////////////////////////////////// + Z_type VrnaHandler:: getRT( const Z_type temperature ) diff --git a/src/IntaRNA/VrnaHandler.h b/src/IntaRNA/VrnaHandler.h index e08aaffa..b5ad46f5 100644 --- a/src/IntaRNA/VrnaHandler.h +++ b/src/IntaRNA/VrnaHandler.h @@ -2,6 +2,7 @@ #define VIENNAHANDLER_H_ #include "IntaRNA/general.h" +#include #include extern "C" { @@ -37,6 +38,10 @@ class VrnaHandler { /** * Construction with global VRNA folding parameter setup + * Complete parameter loading before starting concurrent folding computations. + * For ViennaRNA 2.7 and newer, the first handler also installs a mutex for + * logging without changing output callbacks. Retain this logging lock for + * all subsequent concurrent folding; logger resets remove it. * * @param vrnaParamFile name of a VRNA parameter file to be used for * parameter setup @@ -58,8 +63,8 @@ class VrnaHandler { /** * Generates a new VRNA parameter model according to the global and local settings * - * NOTE: you have to call vrna_md_defaults_reset() to broadcast - * the model details before using non-VRNA-API-3 functions. + * Pass the returned model explicitly to ViennaRNA APIs; do not broadcast it + * through global model defaults while other threads are folding. * * @param max_bp_span Maximal distance between base pair partners within one RNA * or -1 if no constraint needed @@ -100,6 +105,18 @@ class VrnaHandler { double getPfScaleDefault(); + /** + * Provides the process-wide lock for ViennaRNA's shared initialization state. + * Hold this lock while creating parameters and fold compounds, adding + * constraints, and preparing folding. Global MFE/PF calls also require it. + * Prepared window computations may run concurrently with OpenMP-enabled + * ViennaRNA, provided energy parameters are not reloaded during folding. + * @return the mutex shared by all IntaRNA ViennaRNA callers + */ + static + std::mutex & + getSetupMutex(); + }; diff --git a/src/bin/IntaRNA.cpp b/src/bin/IntaRNA.cpp index 555e0d02..45172a6e 100644 --- a/src/bin/IntaRNA.cpp +++ b/src/bin/IntaRNA.cpp @@ -192,7 +192,7 @@ int main(int argc, char **argv){ #endif { VLOG(1) <<"computing accessibility for target '"< targetAcc(parameters.getTargetAccessibility(targetNumber)); INTARNA_CHECK_NOT_NULL(targetAcc.get(),"target initialization failed"); diff --git a/tests/AccessibilityVrna_test.cpp b/tests/AccessibilityVrna_test.cpp index d2110eb1..3e4ff4b9 100644 --- a/tests/AccessibilityVrna_test.cpp +++ b/tests/AccessibilityVrna_test.cpp @@ -8,6 +8,15 @@ #include "IntaRNA/InteractionEnergyVrna.h" #include "IntaRNA/ReverseAccessibility.h" +#include +#include +#include +#include + +#if INTARNA_MULITHREADING +#include +#endif + using namespace IntaRNA; @@ -90,3 +99,100 @@ TEST_CASE("AccessibilityVrna", "[AccessibilityVrna]") { REQUIRE( E_isNotINF(unrestrictedEnergy.getEall2()) ); } } + +TEST_CASE("Concurrent ViennaRNA accessibility and energy setup", "[AccessibilityVrna][threads]") { + + #include "testEasyLoggingSetup.icc" + + // Configure models before starting workers, as the CLI does. Different + // temperatures and window/span sizes expose cross-compound cache reuse. + constexpr size_t caseCount = 12; + constexpr size_t repeats = 4; + constexpr size_t workerCount = 4; + std::vector sequences; + std::vector> handlers; + std::vector> constraints; + sequences.reserve(caseCount); + for (size_t c = 0; c < caseCount; ++c) { + std::string sequence = "GGGGAAAACCCCUAGC"; + for (size_t r = 0; r < c % 3; ++r) { + sequence += "GGGGAAAACCCCUAGC"; + } + sequences.emplace_back("thread-case-" + std::to_string(c), sequence); + handlers.emplace_back(std::make_unique(15.0 + 5.0*c, "Turner04", c % 2, c % 3 == 0)); + std::string structure(sequence.size(), '.'); + if (c % 2) { + structure.front() = structure.back() = 'x'; + structure[sequence.size()/2] = 'b'; + } + constraints.emplace_back(std::make_unique( + sequences.back(), structure, c % 2 ? 10 : 0, "", "", "")); + } + + // Snapshot scientific outputs, including the extra ED band for dangles, + // and global folding paths that can overlap target accessibility in the CLI. + auto calculate = [&](const size_t c) { + const RnaSequence & sequence = sequences[c]; + AccessibilityVrna acc(sequence, 8, constraints[c].get(), *handlers[c], + c % 2 ? 12 : sequence.size(), c % 2 ? 1.2 : 1.07); + ReverseAccessibility reverse(acc); + InteractionEnergyVrna energy(acc, reverse, *handlers[c], 16, 16, true); + std::vector values; + for (size_t i = 0; i < sequence.size(); ++i) { + for (size_t j = i; j < sequence.size(); ++j) { + if (j-i <= 8) { + values.push_back(acc.getED(i,j)); + } + values.push_back(energy.getES1(i,j)); + values.push_back(energy.getES2(i,j)); + } + } + values.push_back(energy.getE_init()); + values.push_back(energy.getEall1()); + values.push_back(energy.getEall2()); + return values; + }; + std::vector> expected(caseCount); + for (size_t c = 0; c < caseCount; ++c) { + expected[c] = calculate(c); + } + + // Catch assertions stay on the calling thread; each job owns its results + // and exception slot, so test bookkeeping adds no data races of its own. + std::vector> actual(caseCount * repeats); + std::vector errors(actual.size()); + auto runJob = [&](const size_t job) { + try { + actual[job] = calculate(job % caseCount); + } catch (...) { + errors[job] = std::current_exception(); + } + }; + + SECTION("native threads also protect builds without IntaRNA OpenMP") { + std::vector workers; + for (size_t worker = 0; worker < workerCount; ++worker) { + workers.emplace_back([&, worker] { + for (size_t job = worker; job < actual.size(); job += workerCount) { + runJob(job); + } + }); + } + // jthread joins before the common assertions below. + } + +#if INTARNA_MULITHREADING + SECTION("OpenMP workers match the CLI execution model") { + #pragma omp parallel for schedule(dynamic) num_threads(workerCount) + for (size_t job = 0; job < actual.size(); ++job) { + runJob(job); + } + } +#endif + + for (size_t job = 0; job < actual.size(); ++job) { + CAPTURE(job); + REQUIRE(errors[job] == nullptr); + REQUIRE(actual[job] == expected[job % caseCount]); + } +} diff --git a/tests/Makefile.am b/tests/Makefile.am index ea9662b2..bba03d63 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -15,7 +15,7 @@ TEST_EXTENSIONS = $(EXEEXT) .sh SH_LOG_COMPILER = $(SHELL) # the script needed for tests -dist_check_SCRIPTS = runIntaRNA.sh runAccessibilityBinary.sh runIntaRNAeval.sh runKineticSeedExtension.sh runOutputOverlap.sh +dist_check_SCRIPTS = runIntaRNA.sh runAccessibilityBinary.sh runAccessibilityThreads.sh runIntaRNAeval.sh runKineticSeedExtension.sh runOutputOverlap.sh # the program to build check_PROGRAMS = runApiTests diff --git a/tests/runAccessibilityThreads.sh b/tests/runAccessibilityThreads.sh new file mode 100644 index 00000000..a007f833 --- /dev/null +++ b/tests/runAccessibilityThreads.sh @@ -0,0 +1,62 @@ +#!/usr/bin/env bash +# Compare complete accessibility output and predictions across worker counts. +set -euo pipefail +bin="$INTARNABINPATH/src/bin/IntaRNA" +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT +export OMP_NUM_THREADS=2 OMP_THREAD_LIMIT=2 +"$bin" --fullhelp > "$tmp/help" +if ! grep -q -- '--threads' "$tmp/help"; then + exit 77 +fi +cat > "$tmp/sequences.fa" <<'EOF' +>short +GGGGAAAACCCCUAGC +>medium +GCUAGGGGUUUUCCCCGCUA +>long +GGGGAAAACCCCUAGCGGGGAAAA +EOF +for i in $(seq 1 16); do + printf '%s 0.5\n' "$i" +done > "$tmp/reactivities.shape" +common=(--query="$tmp/sequences.fa" --target="$tmp/sequences.fa" + --qIntLenMax=8 --tIntLenMax=8 --seedBP=3 --qAccW=14 --tAccW=18 + --qAccL=10 --tAccL=12 + --outMode=C + --outMaxE=100 --default-log-file=/dev/null) +# D converts reactivities; W reads precomputed pseudoenergies. See the audit +# for pre-existing serial failures of Z and W combined with global Eall output. +for method in none D W; do + shape=() + columns=id1,start1,end1,id2,start2,end2,E,ED1,ED2 + if test "$method" != W; then + columns+=,Eall1,Eall2 + fi + if test "$method" != none; then + shape=(--qShape="$tmp/reactivities.shape" --tShape="$tmp/reactivities.shape" + --qShapeMethod="$method" --tShapeMethod="$method") + fi + for threads in 1 2; do + mkdir "$tmp/$method-$threads" + "$bin" "${common[@]}" "${shape[@]}" --threads="$threads" --outCsvCols="$columns" \ + --out="qAcc:$tmp/$method-$threads/query.txt" \ + --out="tAcc:$tmp/$method-$threads/target.txt" \ + > "$tmp/$method-$threads/predictions.unsorted" || { + cat "$tmp/$method-$threads/predictions.unsorted" + exit 1 + } + # Workers may publish sequence pairs in a different order. + LC_ALL=C sort "$tmp/$method-$threads/predictions.unsorted" \ + > "$tmp/$method-$threads/predictions" + test "$(wc -l < "$tmp/$method-$threads/predictions")" -gt 1 + done + cmp "$tmp/$method-1/predictions" "$tmp/$method-2/predictions" + for strand in query target; do + for sequence in 1 2 3; do + cmp "$tmp/$method-1/$strand-s$sequence.txt" \ + "$tmp/$method-2/$strand-s$sequence.txt" + done + done +done +echo 'Parallel accessibility, SHAPE and prediction checks passed' From 2d20f8055fc2f501f4226da38b9ed7e01c820a22 Mon Sep 17 00:00:00 2001 From: AlexanderMitrofanov Date: Wed, 14 Oct 2026 18:48:29 +0200 Subject: [PATCH 2/4] Keep accessibility concurrency tests portable to Apple Clang --- ChangeLog | 10 ++++++++++ tests/AccessibilityVrna_test.cpp | 28 ++++++++++++++++++++-------- tests/runAccessibilityThreads.sh | 8 ++++---- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 18bc7d4b..bc92b8f1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -45,6 +45,8 @@ - fix ViennaRNA parameter-cache races and enable parallel prepared window accessibility; retain serialization for global MFE/PF and non-OpenMP ViennaRNA builds, with a source audit and concurrency regressions (issue #265) +- keep the concurrency regression compatible with Apple Clang's standard + library using explicitly joined std::thread workers (PR #267) - add an API overview, IntaRNA Doxygen styling, and automated documentation builds and GitHub Pages publishing alongside the user guide (issue #22) @@ -133,6 +135,14 @@ energies, restricted partition sums, and trackers. ################################################################################ ################################################################################ +261014 Alexander Mitrofanov + * tests/AccessibilityVrna_test.cpp : + * replace std::jthread, unavailable in the macOS CI standard library, with + std::thread and exception-safe joins before inspecting worker results + * fixes the Apple Clang regression build in PR #267 + * tests/runAccessibilityThreads.sh : + * avoid empty-array expansion with nounset for older macOS Bash versions + 261014 Alexander Mitrofanov * IntaRNA/VrnaHandler, IntaRNA/AccessibilityVrna, IntaRNA/InteractionEnergyVrna, bin/IntaRNA : diff --git a/tests/AccessibilityVrna_test.cpp b/tests/AccessibilityVrna_test.cpp index 3e4ff4b9..452ccdf8 100644 --- a/tests/AccessibilityVrna_test.cpp +++ b/tests/AccessibilityVrna_test.cpp @@ -170,15 +170,27 @@ TEST_CASE("Concurrent ViennaRNA accessibility and energy setup", "[Accessibility }; SECTION("native threads also protect builds without IntaRNA OpenMP") { - std::vector workers; - for (size_t worker = 0; worker < workerCount; ++worker) { - workers.emplace_back([&, worker] { - for (size_t job = worker; job < actual.size(); job += workerCount) { - runJob(job); - } - }); + // Apple Clang's supported standard library does not provide jthread. + // Join every launched worker even if creating a later thread fails. + std::vector workers; + workers.reserve(workerCount); + try { + for (size_t worker = 0; worker < workerCount; ++worker) { + workers.emplace_back([&, worker] { + for (size_t job = worker; job < actual.size(); job += workerCount) { + runJob(job); + } + }); + } + } catch (...) { + for (auto & worker : workers) { + worker.join(); + } + throw; + } + for (auto & worker : workers) { + worker.join(); } - // jthread joins before the common assertions below. } #if INTARNA_MULITHREADING diff --git a/tests/runAccessibilityThreads.sh b/tests/runAccessibilityThreads.sh index a007f833..ac91ec4b 100644 --- a/tests/runAccessibilityThreads.sh +++ b/tests/runAccessibilityThreads.sh @@ -28,18 +28,18 @@ common=(--query="$tmp/sequences.fa" --target="$tmp/sequences.fa" # D converts reactivities; W reads precomputed pseudoenergies. See the audit # for pre-existing serial failures of Z and W combined with global Eall output. for method in none D W; do - shape=() + command=("$bin" "${common[@]}") columns=id1,start1,end1,id2,start2,end2,E,ED1,ED2 if test "$method" != W; then columns+=,Eall1,Eall2 fi if test "$method" != none; then - shape=(--qShape="$tmp/reactivities.shape" --tShape="$tmp/reactivities.shape" - --qShapeMethod="$method" --tShapeMethod="$method") + command+=(--qShape="$tmp/reactivities.shape" --tShape="$tmp/reactivities.shape" + --qShapeMethod="$method" --tShapeMethod="$method") fi for threads in 1 2; do mkdir "$tmp/$method-$threads" - "$bin" "${common[@]}" "${shape[@]}" --threads="$threads" --outCsvCols="$columns" \ + "${command[@]}" --threads="$threads" --outCsvCols="$columns" \ --out="qAcc:$tmp/$method-$threads/query.txt" \ --out="tAcc:$tmp/$method-$threads/target.txt" \ > "$tmp/$method-$threads/predictions.unsorted" || { From d403182ff8d56f02ed47309190a104c1ca1dbd3b Mon Sep 17 00:00:00 2001 From: AlexanderMitrofanov Date: Thu, 15 Oct 2026 15:53:18 +0200 Subject: [PATCH 3/4] Require ViennaRNA 2.7.0 for synchronized logging --- AGENTS.md | 8 +- ChangeLog | 18 ++++ README.md | 11 +- conda-build-env.yml | 2 +- configure.ac | 41 +++++-- .../vienna-accessibility-thread-safety.md | 102 +++++++++++++----- src/IntaRNA/AccessibilityVrna.cpp | 3 +- src/IntaRNA/VrnaHandler.cpp | 6 -- src/IntaRNA/VrnaHandler.h | 4 +- 9 files changed, 143 insertions(+), 52 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c39f3177..ca805bc7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,7 @@ Apple Clang release on macOS. C++23 support is required in both the compiler and its standard library. `configure` checks `std::string::contains`, `std::string::resize_and_overwrite` and `std::stringstream::view`. Do not lower the language standard to work around -an old toolchain. Dependencies include Boost, ViennaRNA (>= 2.4.14), zlib, +an old toolchain. Dependencies include Boost, ViennaRNA (>= 2.7.0), zlib, pkg-config and Autotools; OpenMP is used by the default multithreaded build. [conda-build-env.yml](conda-build-env.yml) supplies library/build dependencies, but does not select the C++ compiler. Use the platform setup in CI when needed. @@ -154,8 +154,10 @@ Example::isComplete() const setup, constraints and preparation use `VrnaHandler::getSetupMutex()`. Prepared window accessibility computations can run concurrently with an OpenMP-enabled ViennaRNA build; global MFE/PF calls remain serialized. - Complete parameter loading before workers start, preserve the ViennaRNA - logger lock, and consult `doc/analysis/vienna-accessibility-thread-safety.md` + Complete parameter loading before workers start and preserve the mandatory + ViennaRNA logger lock. Retain full-call serialization for RNAlib builds + without OpenMP; this is a build-configuration boundary, not support for old + ViennaRNA releases. Consult `doc/analysis/vienna-accessibility-thread-safety.md` before changing these boundaries. - For recurrence or predictor changes, extend the relevant regression tests and short-sequence oracles (`PredictorTinyOracle_test.cpp` and diff --git a/ChangeLog b/ChangeLog index bc92b8f1..156d3ba2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -42,6 +42,9 @@ ## Technical changes and Optimizations +- require ViennaRNA >= 2.7.0 and its logger/prepared-window APIs at configure + time; install the logger lock for every supported version (PR #267) + - fix ViennaRNA parameter-cache races and enable parallel prepared window accessibility; retain serialization for global MFE/PF and non-OpenMP ViennaRNA builds, with a source audit and concurrency regressions (issue #265) @@ -135,6 +138,21 @@ energies, restricted partition sums, and trackers. ################################################################################ ################################################################################ +261015 Alexander Mitrofanov + * configure.ac, conda-build-env.yml : + * require ViennaRNA >= 2.7.0, the first release with vrna_log_lock_set() + * check header version and link the logger/prepared-window APIs during + configure, including builds using --disable-pkg-config + * IntaRNA/VrnaHandler, IntaRNA/AccessibilityVrna : + * install the logger lock unconditionally for supported ViennaRNA releases + and remove obsolete version guards; retain full-call serialization for + RNAlib builds without OpenMP + * README.md, AGENTS.md, doc/analysis/vienna-accessibility-thread-safety.md : + * align the dependency and synchronization contract; distinguish historical + 2.4.14 source evidence from the supported range and recorded runtime tests + + record minimum-version, release/debug and dependency-probe validation + * addresses review of https://github.com/BackofenLab/IntaRNA/pull/267 + 261014 Alexander Mitrofanov * tests/AccessibilityVrna_test.cpp : * replace std::jthread, unavailable in the macOS CI standard library, with diff --git a/README.md b/README.md index 211d4943..0d67fe7c 100644 --- a/README.md +++ b/README.md @@ -194,7 +194,10 @@ dependencies: - libboost_system - libboost_iostreams - libboost_serialization -- [Vienna RNA package](http://www.tbi.univie.ac.at/RNA/) version >= 2.4.14 +- [Vienna RNA package](http://www.tbi.univie.ac.at/RNA/) version >= 2.7.0, + providing the logger locking API required for concurrent accessibility + calculations. Configure checks the version and required APIs, including + when `--disable-pkg-config` is used. - `pkg-config` for detailed version checks of dependencies - if [cloning from github](#instgithub): GNU autotools (automake, autoconf, ..) @@ -460,8 +463,10 @@ If `--threads=k != 1`, than *k* predictions are processed in parallel. A value o ViennaRNA accessibility calculations for independent sequences run concurrently when ViennaRNA was built with OpenMP support. IntaRNA serializes parameter and constraint setup to protect ViennaRNA's shared caches; builds without ViennaRNA -OpenMP support retain serial accessibility calculation. Global intramolecular -MFE/partition-function calls remain serialized. See the +OpenMP support retain serial accessibility calculation. IntaRNA installs a +separate ViennaRNA logger lock during handler initialization for every +supported version. Global intramolecular MFE/partition-function calls remain +serialized. See the [thread-safety audit](doc/analysis/vienna-accessibility-thread-safety.md) for the call sequence, source evidence, standalone reproducer and validation. diff --git a/conda-build-env.yml b/conda-build-env.yml index a038bb05..7f75876a 100644 --- a/conda-build-env.yml +++ b/conda-build-env.yml @@ -12,7 +12,7 @@ dependencies: - pkgconfig - zlib - boost-cpp - - viennarna>=2.4.14 + - viennarna>=2.7.0 - cxx-compiler - make - binutils diff --git a/configure.ac b/configure.ac index ae2df6d9..aeb4e0c9 100644 --- a/configure.ac +++ b/configure.ac @@ -5,8 +5,8 @@ AC_INIT([IntaRNA], [3.4.1], [], [intaRNA], [http://www.bioinf.uni-freiburg.de] ) # minimal required version of the boost library BOOST_REQUIRED_VERSION=1.50.0 -# minimal required version of the Vienna RNA library -VRNA_REQUIRED_VERSION=2.4.14 +# minimal Vienna RNA version providing the logger locking API +VRNA_REQUIRED_VERSION=2.7.0 # minimal required version of python interpreter (to install intarnapvalue) PYTHON_REQUIRED_VERSION=3.6 @@ -443,19 +443,41 @@ AS_IF([test x"$HAVE_PKG_CONFIG" = x"yes"], [ ]) AS_IF([test "$RNANOTFOUND" = "0"], [ - AC_MSG_CHECKING([for compilation with Vienna RNA package headers]) + AC_MSG_CHECKING([for ViennaRNA >= $VRNA_REQUIRED_VERSION with logger locking and prepared window folding]) OLD_CPPFLAGS=$CPPFLAGS - OLD_CXXFLAGS=$CXXFLAGS OLD_LDFLAGS=$LDFLAGS - AC_LANG_PUSH([C]) + intarna_saved_vrna_LIBS=$LIBS + AC_LANG_PUSH([C++]) CPPFLAGS="$CPPFLAGS $AM_CXXFLAGS" LDFLAGS="$LDFLAGS $AM_LDFLAGS" AS_IF([test "x$HAVE_PKG_CONFIG" = "xyes"], [ CPPFLAGS="$CPPFLAGS $VRNA_CFLAGS" - LDFLAGS="$LDFLAGS $VRNA_LIBS" + LIBS="$VRNA_LIBS $LIBS" + ], [ + LIBS="-lRNA $LIBS" ]) - AC_COMPILE_IFELSE( - [AC_LANG_PROGRAM([[#include ]],[[vrna_md_t tmp; vrna_md_copy(&tmp,&tmp);]])], + # Check headers even without pkg-config, and link the APIs used for concurrency. + AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[ +extern "C" { +#include +#if VRNA_VERSION_MAJOR < 2 || (VRNA_VERSION_MAJOR == 2 && VRNA_VERSION_MINOR < 7) +#error ViennaRNA 2.7.0 or newer is required +#endif +#include +#include +#include +#include +} +]], [[ +vrna_md_t model; +vrna_md_set_default(&model); +vrna_log_lock_set(NULL, NULL); +vrna_fold_compound_t *fc = vrna_fold_compound("GCGC", &model, VRNA_OPTION_PF | VRNA_OPTION_WINDOW); +vrna_fold_compound_prepare(fc, VRNA_OPTION_PF | VRNA_OPTION_WINDOW); +vrna_probs_window(fc, 4, VRNA_PROBS_WINDOW_UP, NULL, NULL); +vrna_fold_compound_free(fc); +]])], [ AC_MSG_RESULT([yes]) RNANOTFOUND=0; @@ -467,9 +489,10 @@ AS_IF([test "$RNANOTFOUND" = "0"], [ RNANOTFOUND=1; ] ) - AC_LANG_POP([C]) + AC_LANG_POP([C++]) CPPFLAGS=$OLD_CPPFLAGS LDFLAGS=$OLD_LDFLAGS + LIBS=$intarna_saved_vrna_LIBS ]) # error output if ViennaRNA not found diff --git a/doc/analysis/vienna-accessibility-thread-safety.md b/doc/analysis/vienna-accessibility-thread-safety.md index 7f5668d0..a0cb70ef 100644 --- a/doc/analysis/vienna-accessibility-thread-safety.md +++ b/doc/analysis/vienna-accessibility-thread-safety.md @@ -8,15 +8,26 @@ uses compound-owned state and can run concurrently after synchronized setup. ## Reviewed upstream sources -The review covers the minimum supported ViennaRNA release, **2.4.14**, and the -**2.7.2** release used for the runtime experiments. Source links below are -pinned to immutable commits: +IntaRNA requires **ViennaRNA 2.7.0 or later**. The review covers that release's +required logging API and the **2.7.2** implementation used for the runtime +experiments. The earlier **2.4.14** review remains as historical evidence; +that release is no longer supported. Source links below are pinned to immutable +commits: | Release | Commit | Scope | | --- | --- | --- | -| 2.4.14 | [`0b8ca4c6604e7a8bea366af62d82f8f40cea49bf`](https://github.com/ViennaRNA/ViennaRNA/tree/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf) | Minimum supported API and implementation | +| 2.4.14 | [`0b8ca4c6604e7a8bea366af62d82f8f40cea49bf`](https://github.com/ViennaRNA/ViennaRNA/tree/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf) | Historical implementation comparison; unsupported | +| 2.7.0 | [`219394580aec203a9d6f0d5450021e22642d5a83`](https://github.com/ViennaRNA/ViennaRNA/tree/219394580aec203a9d6f0d5450021e22642d5a83) | Minimum supported release and public logger locking API | | 2.7.2 | [`1ffec79f5e258896160f7362ced8263450f371dc`](https://github.com/ViennaRNA/ViennaRNA/tree/1ffec79f5e258896160f7362ced8263450f371dc) | Installed runtime and current parameter cache implementation | +ViennaRNA 2.7.0 introduced the flexible logger, as recorded in its +[release notes](https://github.com/ViennaRNA/ViennaRNA/releases/tag/v2.7.0). +Its public [`vrna_log_lock_set()` interface](https://github.com/ViennaRNA/ViennaRNA/blob/219394580aec203a9d6f0d5450021e22642d5a83/src/ViennaRNA/utils/log.h#L379-L398) +lets IntaRNA protect diagnostics even when RNAlib has no built-in pthread +locking. This interface determines the minimum supported version. Configure +checks the header version and links the required logger and prepared-window +APIs. Using `--disable-pkg-config` follows the same requirement. + This is a review of the paths IntaRNA calls, not a blanket claim that RNAlib, its deprecated interfaces, or every future release is thread-safe. In particular, compiling RNAlib with OpenMP does not make all its static state @@ -70,7 +81,7 @@ and [matrix preparation](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e2 | State or operation | Evidence | Consequence | | --- | --- | --- | | Parameter caches `p_pre`, `pf_pre`, and their initialization flags | [2.7.2 `params.c`](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/params/params.c#L88-L217) enables `SPEEDUP_PARAMS`; both constructors rewrite cached model members and copy whole cached structures. Only `id` and `pf_id` are declared OpenMP thread-private. | Guard every IntaRNA entry to parameter construction with one shared mutex. Separate locks for accessibility and energy construction would not protect the same caches. | -| Parameter identifiers in older RNAlib | [2.4.14 declarations](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/params/params.c#L88-L98), [identifier increment](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/params/params.c#L552) | 2.4.14 lacks the newer caches, but its identifier is shared in builds without OpenMP. The same setup lock remains useful. | +| Parameter identifiers in historical RNAlib | [2.4.14 declarations](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/params/params.c#L88-L98), [identifier increment](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/params/params.c#L552) | The unsupported 2.4.14 release lacks the newer caches, but its identifier is shared in builds without OpenMP. This explains why removing the setup lock was not safe under the former version requirement. | | Global model defaults and legacy model variables | [2.7.2 `vrna_md_defaults_reset`](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/model.c#L346-L432), [2.4.14 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/model.c#L311-L384) | Passing a model to `vrna_md_defaults_reset()` changes process-wide defaults; it does not merely reset that local object. Remove the worker-side call when every fold already receives its explicit model. Guard parameter-file loading/default reads during handler setup. | | `vrna_md_copy` | [2.7.2 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/model.c#L196-L232) copies the supplied structure and its arrays. | Copying a stable handler model into worker-local storage needs no global lock. | | Sliding-window DP, auxiliary arrays, and constraint updates | [2.7.2 window implementation](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/partfunc/pf_window.c#L681-L958), [2.4.14 implementation](https://github.com/ViennaRNA/ViennaRNA/blob/0b8ca4c6604e7a8bea366af62d82f8f40cea49bf/src/ViennaRNA/LPfold.c#L669-L941) obtain arrays and models from the compound or allocate local helpers. | Independent prepared compounds can compute concurrently. Keep callback storage distinct and alive until each call returns. | @@ -92,32 +103,34 @@ IntaRNA itself is built without OpenMP. The parallel boundary is **create, constrain, and prepare while locked; compute the window recurrence with exclusive ownership of the compound**. Keep global MFE/PF calls synchronized. RNAlib builds without -OpenMP use a conservative full-call fallback. Do not concurrently mutate -the handler model, reload global parameter tables through external RNAlib -calls, modify the same compound, or replace callback state while a call is -running. IntaRNA's mutex cannot coordinate direct RNAlib calls made by other -libraries in an embedding application. +OpenMP use conservative full-call serialization. This is a property of the +RNAlib build configuration and is retained independently of the minimum +version. Do not concurrently mutate the handler model, reload global parameter +tables through external RNAlib calls, modify the same compound, or replace +callback state while a call is running. IntaRNA's mutex cannot coordinate +direct RNAlib calls made by other libraries in an embedding application. This guarantee concerns the ViennaRNA boundary. Native-thread library users must separately provide thread-safe IntaRNA/easylogging++ diagnostics, or leave verbose/performance logging disabled. The CLI's OpenMP logging synchronization is not a general native-thread logging contract. -For ViennaRNA 2.7 and later, handler initialization installs a process-lifetime -mutex callback once through `vrna_log_lock_set()`. This replaces the logger's -locking hook while preserving its output streams, levels, and message -callbacks. The logger mutex is distinct from the setup mutex because guarded -ViennaRNA operations can emit messages themselves. This protects diagnostics -even if RNAlib was built without pthread support. Embedding applications must -not reset or replace that hook while IntaRNA folds are running; callbacks must -not reenter folding. Earlier reviewed logging uses stdio directly rather than -the newer lazily initialized callback array. See the +For every supported ViennaRNA version, handler initialization installs a +process-lifetime mutex callback once through `vrna_log_lock_set()`. This +replaces the logger's locking hook while preserving its output streams, levels, +and message callbacks. The logger mutex is distinct from the setup mutex +because guarded ViennaRNA operations can emit messages themselves. This +protects diagnostics even if RNAlib was built without pthread support. +Embedding applications must not reset or replace that hook while IntaRNA folds +are running; callbacks must not reenter folding. There is no version-dependent +path that omits the logger lock. See the [public locking callback](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/utils/log.h#L61-L70) and [hook setter](https://github.com/ViennaRNA/ViennaRNA/blob/1ffec79f5e258896160f7362ced8263450f371dc/src/ViennaRNA/utils/log.c#L284-L290). -The 2.4.14 source review establishes API availability and identifies its -state, but does not substitute for testing that release. Runtime comparisons -on 2.7.2 do not prove every intermediate release or build configuration safe. +The original runtime comparisons below used 2.7.2. The dependency-floor +revision also tested 2.7.0, as recorded below. Neither these finite tests nor +the historical 2.4.14 review establish runtime behavior for every supported +release or build configuration. Repeat the audit when changing the supported RNAlib range, adding model mutations after preparation, changing SHAPE integration, or adding callbacks. @@ -159,7 +172,8 @@ them as separate diagnostic processes, never as ordinary CI pass conditions: Observed results used ViennaRNA 2.7.2's static library with OpenMP enabled, GCC 14.4.0 (conda-forge build 14.4.0-7), and Linux x86-64 with CPU affinity 0-11. Runs requested eight workers; other build activity could share these -CPUs. No ViennaRNA 2.4.14 runtime was installed for the experiment. +CPUs. These recorded experiments did not test ViennaRNA 2.7.0 or 2.4.14 +runtimes. | Mode/profile | Completed repetitions | Exact probability comparisons | Result | | --- | ---: | ---: | --- | @@ -180,10 +194,11 @@ not a race-detector proof or a performance measurement. ## Integrated regressions and existing SHAPE limitations -GCC 14 release and `--enable-debug` builds each passed `make tests -j4`: -all seven test programs/scripts passed, including 75,901 API assertions in -69 test cases. The source base was `c5f1d51` on `master` (the repository has no -`main` branch). `git diff --check` and shell syntax validation also passed. +For the original synchronization change, GCC 14 release and `--enable-debug` +builds each passed `make tests -j4`: all seven test programs/scripts passed, +including 75,901 API assertions in 69 test cases. The source base was `c5f1d51` +on `master` (the repository has no `main` branch). `git diff --check` and shell +syntax validation also passed. `tests/AccessibilityVrna_test.cpp` compares all retained ED values (including the extra band used for dangling ends), ES matrices, initiation energies and @@ -227,6 +242,39 @@ both the parallel and serialized branches. It is **not** a runtime test of a non-OpenMP RNAlib: the linked library remained the installed OpenMP-enabled 2.7.2, and unchanged IntaRNA objects came from the release archive. +## Dependency-floor revision validation (2026-10-15) + +The revision requested in [PR #267](https://github.com/BackofenLab/IntaRNA/pull/267#issuecomment-6192018595) +was validated on Linux with GCC 14.4.0: + +| ViennaRNA | IntaRNA build | Command | Result | +| --- | --- | --- | --- | +| 2.7.0 | Release | `make tests -j4` | All seven suites passed | +| 2.7.2 | Release | `make tests -j2` | All seven suites passed | +| 2.7.2 | Debug | `make tests -j2` | All seven suites passed | + +Each run passed 75,901 API assertions in 69 cases, including the native-thread +and OpenMP accessibility comparisons. The 2.7.0 standalone diagnostic, +`vrna-window-threads setup-locked 4 3`, compared 1,011,504 probabilities over +24 mixed-model cases and three repetitions, with no parameter, probability, +or callback metadata mismatches. + +ViennaRNA 2.7.0 was built from its official release tarball as a static library +with OpenMP and pthread support. Its library-only, SVM-disabled build needed +`make 'libRNA_utils_la_CPPFLAGS=$(AM_CPPFLAGS)'` to restore an upstream omitted +include path; no upstream source was changed. Include and library search paths +placed this installation ahead of the existing 2.7.2 dependency prefix. The +IntaRNA executable's version output and compiler dependency files confirmed +that the minimum-version headers were selected. + +Six isolated configure probes also passed: a valid installation with +`--disable-pkg-config` and explicit static-library dependencies was accepted; +old pkg-config metadata, headers reporting 2.6.4 with and without pkg-config, +a missing logger declaration, and an unavailable logger link symbol were +rejected. These negative checks used controlled fixtures, not an installed +2.6.4 runtime. Installation, an external consumer exercising accessibility, +all 69 standalone public-header checks, and `git diff --check` passed. + ## End-to-end measurement The base and corrected binaries used the same GCC 14.4 release toolchain, diff --git a/src/IntaRNA/AccessibilityVrna.cpp b/src/IntaRNA/AccessibilityVrna.cpp index 8d53c829..d6f7ef1a 100644 --- a/src/IntaRNA/AccessibilityVrna.cpp +++ b/src/IntaRNA/AccessibilityVrna.cpp @@ -244,7 +244,8 @@ fillByRNAplfold( const VrnaHandler &vrnaHandler #ifdef VRNA_WITH_OPENMP // The expensive window recurrence and callback use per-compound storage. - // Retain full serialization for ViennaRNA builds without OpenMP support. + // OpenMP is a ViennaRNA build option, independent of its version; keep + // full serialization when the linked library was built without it. setupLock.unlock(); #endif diff --git a/src/IntaRNA/VrnaHandler.cpp b/src/IntaRNA/VrnaHandler.cpp index 594af3c6..79777843 100644 --- a/src/IntaRNA/VrnaHandler.cpp +++ b/src/IntaRNA/VrnaHandler.cpp @@ -16,14 +16,11 @@ extern "C" { #include #include #include -#if VRNA_VERSION_MAJOR > 2 || (VRNA_VERSION_MAJOR == 2 && VRNA_VERSION_MINOR >= 7) #include -#endif } namespace { -#if VRNA_VERSION_MAJOR > 2 || (VRNA_VERSION_MAJOR == 2 && VRNA_VERSION_MINOR >= 7) void lockVrnaLogging( const int lock, void * ) noexcept { @@ -36,7 +33,6 @@ lockVrnaLogging( const int lock, void * ) noexcept loggingMutex.unlock(); } } -#endif } @@ -55,11 +51,9 @@ VrnaHandler( Z_type temperature { std::lock_guard setupLock(getSetupMutex()); -#if VRNA_VERSION_MAJOR > 2 || (VRNA_VERSION_MAJOR == 2 && VRNA_VERSION_MINOR >= 7) // Install only the locking callback; preserve log outputs and user callbacks. static std::once_flag loggingInitialized; std::call_once(loggingInitialized, [] { vrna_log_lock_set(lockVrnaLogging, nullptr); }); -#endif // load parameters if ( !vrnaParamFile.empty() ) { diff --git a/src/IntaRNA/VrnaHandler.h b/src/IntaRNA/VrnaHandler.h index b5ad46f5..4e3ea4f7 100644 --- a/src/IntaRNA/VrnaHandler.h +++ b/src/IntaRNA/VrnaHandler.h @@ -39,8 +39,8 @@ class VrnaHandler { /** * Construction with global VRNA folding parameter setup * Complete parameter loading before starting concurrent folding computations. - * For ViennaRNA 2.7 and newer, the first handler also installs a mutex for - * logging without changing output callbacks. Retain this logging lock for + * The first handler also installs a mutex for logging without changing + * output callbacks. Retain this logging lock for * all subsequent concurrent folding; logger resets remove it. * * @param vrnaParamFile name of a VRNA parameter file to be used for From 0a55f0005ca7aa2bcd2b08d6fb1ab803b2aa58ab Mon Sep 17 00:00:00 2001 From: Martin Raden Date: Fri, 16 Oct 2026 11:25:22 +0200 Subject: [PATCH 4/4] Update EXTRA_DIST in Makefile.am Removed unnecessary files from EXTRA_DIST in Makefile. --- doc/Makefile.am | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index 6919154c..cb78b081 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -4,8 +4,6 @@ ################################################################ EXTRA_DIST = \ - analysis/vienna-accessibility-thread-safety.md \ - analysis/vrna-window-threads.cpp \ conda.txt \ api-documentation.md \ build-api.sh \