diff --git a/AGENTS.md b/AGENTS.md index 46333509..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. @@ -150,8 +150,15 @@ 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 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 `PredictorSeedOracle_test.cpp`). For optimizations, compare correctness first, diff --git a/ChangeLog b/ChangeLog index de36b63d..156d3ba2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -42,6 +42,15 @@ ## 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) +- 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) @@ -129,6 +138,48 @@ 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 + 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 : + * 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..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, ..) @@ -457,6 +460,16 @@ 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. 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. + When using parallelization, you should have the following in mind: - The memory consumption will be multiplied by the number of threads, 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 new file mode 100644 index 00000000..a0cb70ef --- /dev/null +++ b/doc/analysis/vienna-accessibility-thread-safety.md @@ -0,0 +1,341 @@ +# 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 + +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) | 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 +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 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. | +| 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 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 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 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. + +## 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. These recorded experiments did not test ViennaRNA 2.7.0 or 2.4.14 +runtimes. + +| 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 + +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 +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. + +## 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, +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..d6f7ef1a 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,28 @@ 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. + // OpenMP is a ViennaRNA build option, independent of its version; keep + // full serialization when the linked library was built without it. + 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..79777843 100644 --- a/src/IntaRNA/VrnaHandler.cpp +++ b/src/IntaRNA/VrnaHandler.cpp @@ -16,6 +16,24 @@ extern "C" { #include #include #include + #include +} + +namespace { + +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(); + } +} + } namespace IntaRNA { @@ -31,6 +49,11 @@ VrnaHandler( Z_type temperature model() , RT(getRT(temperature)) { + std::lock_guard setupLock(getSetupMutex()); + + // 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); }); // load parameters if ( !vrnaParamFile.empty() ) { @@ -142,6 +165,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..4e3ea4f7 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. + * 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..452ccdf8 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,112 @@ 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") { + // 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(); + } + } + +#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..ac91ec4b --- /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 + 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 + command+=(--qShape="$tmp/reactivities.shape" --tShape="$tmp/reactivities.shape" + --qShapeMethod="$method" --tShapeMethod="$method") + fi + for threads in 1 2; do + mkdir "$tmp/$method-$threads" + "${command[@]}" --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'