If Pillow or numpy aren't importable, fail fast with a clear
**ERROR** message and pip-install hint instead of a bare traceback
from inside pixel_diff().
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Three independent issues made all 72 musrview-* ctest targets fail on a
RHEL9 box while passing elsewhere:
- Pixel-diff tolerance (1%) was tighter than needed; bump to 3%. Real
diffs against the committed reference PNGs measure ~0.08%.
- find_package(Python3) picks the highest-versioned interpreter on the
system, which on this machine is a bare /usr/bin/python3.14 without
Pillow/numpy installed, while the distro's python3 (3.9) has both.
Probe the interpreter CMake found and fall back to PATH's "python3"
if it lacks the required modules.
- musrview reliably writes and closes the PNG before exiting, but under
ctest (not reproducible via a plain shell loop) the new directory
entry could take several seconds to become visible to the checking
script's glob(). Poll for up to ~15s instead of failing on the first
empty glob.
Since the Red/Green mode handling was added for NeXus HDF5 IDF V2, the
histogram numbers of the routed histograms carry the period offset, i.e.
histoNo = period*10000 + histo. PRunBase::DeadTimeCorrection() was still
addressing the dead time parameter vector with 'histoNo-1', hence for any
period > 0 it read far beyond the end of the vector. The garbage picked up
there scaled the counts, so every invocation of musrfit gave a different
(sometimes nan) result for the affected run blocks.
The dead time parameters are stored contiguously, period-by-period, and
therefore need to be addressed as period*noOfHistosPerPeriod + histo - 1.
Introduce PERIOD_HISTO_OFFSET for the period encoding, keep the number of
histos per period in PRawRunData, and add PRawRunData::GetDeadTimeParam(histoNo)
which does the mapping and guards against out-of-range access.
Cross-checked against the HDF4 IDF V1 twin of the same run, where the two
periods are flattened into histos 1-192: both readers now yield bit-identical
chisq, with and without dead time correction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MacPorts nests libomp's headers/library under an extra 'libomp'
subdirectory and installs the Qt6 CMake package configs under
libexec/qt6 rather than lib/cmake, so find_package(OpenMP)/find_package(Qt6...)
did not see them without manually injecting OpenMP_ROOT/CMAKE_PREFIX_PATH
on the command line. Detect the MacPorts prefix (/opt/local) automatically,
same as already done for Homebrew, including in the install RPATH fallback.
Fixes found while reviewing the cmake setup, with a focus on macOS. All
changes are either platform neutral or guarded by if (APPLE).
Portability / correctness:
* CMP0167 was guarded by 'CMAKE_VERSION GREATER_EQUAL 3.3', a *numeric*
comparison which is true on e.g. cmake 3.22. Since the policy only exists
since cmake 3.30, configuring aborted on RHEL 9 / Ubuntu 22.04, although
cmake_minimum_required allows them. Use 'if (POLICY CMP0167)'.
* CMAKE_REQUIRED_INCLUDES expects directories, not header names. Setting it
to 'math.h' injected a bogus -I<scratch>/math.h into every check_*() and
leaked into the find_package() calls. Dropped, and CMAKE_REQUIRED_LIBRARIES
is now unset after the checks. config.h is unchanged.
* The Qt3 fallback tested 'AND Qt4_FOUND' instead of 'AND NOT Qt4_FOUND'.
* NeXus regression tests were registered unconditionally and therefore failed
in a default build. They are gated on 'nexus' now, and the HDF4 based ones
additionally on HAVE_HDF4.
* HAVE_HDF4 was queried but never defined, i.e. there was no way to switch on
the HDF4 support that PNeXus/PRunDataHandler/dump_header/musrfit implement.
Added option(HAVE_HDF4), to be used together with -Dnexus=1. This is not
academic: the ISIS example data are HDF4 files and test-histo-NeXus only
passes with -DHAVE_HDF4=1.
Do not use ${ROOT_USE_FILE}:
It overwrote CMAKE_CXX_STANDARD with ROOT's value and added ROOT's include
dirs, definitions and recommended compiler flags to *every* target,
including the Qt applications which do not use ROOT at all. Replaced by an
explicit equivalent; the C++ standard is now only raised, never silently
changed. The Qt sub-trees reset flags and include dirs again, mupp/plotter
puts them back since it does use ROOT. ROOT's flags are kept wherever ROOT
headers are used, -fsigned-char and -fsized-deallocation are ABI relevant
there. Verified: the compile flags of all ROOT consuming targets are
byte-identical to before.
Homebrew header shadowing:
Boost_INCLUDE_DIRS and FFTW3_INCLUDE are both /opt/homebrew/include on a
Homebrew system, i.e. the whole 3rd party header tree, and they were passed
to target_include_directories() with BEFORE - ahead of musrfit's own
src/include. They now come from the Boost::headers and FFTW3::FFTW3
imported targets, which contribute them as SYSTEM includes after the
project's own directories.
macOS:
* mupp: do not set the VERSION property on a MACOSX_BUNDLE target. cmake then
names the binary Contents/MacOS/mupp-1.1.0 and only leaves a symlink 'mupp'
beside it, contradicting CFBundleExecutable and not surviving
zipping/codesigning. This is the reason the symlink had to be created by
hand so far. VERSION is still set for the non-APPLE build.
* All four app bundles had an empty CFBundleShortVersionString and
CFBundleVersion; set MACOSX_BUNDLE_SHORT_VERSION_STRING / _BUNDLE_VERSION.
* APPLE is UNIX, so the RPM generator was also active on macOS -> restrict to
'UNIX AND NOT APPLE'. Likewise the /usr/local/lib rpath, which on Apple
silicon should be /opt/homebrew/lib; macOS gets its own branch now.
* mupp: -fpermissive is a GCC option, hand it to GCC only.
* musredit: use VERSION_GREATER_EQUAL for the Darwin version icon switch.
Verified on macOS 26 / arm64 with ROOT 6.40.03, Qt 6.11, cmake 4.4: full
build clean in the default configuration, with -DASlibs -DBMWlibs -DBNMRlibs
-DDummyUserFcn (identical list of 55 targets), and with -Dnexus / -DHAVE_HDF4.
ctest shows no regression at any step; no maxLH test fails in any of the three
NeXus configurations.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rename data members to the f-prefixed CamelCase convention, the
nested helper struct to PHighlightingRule (P instead of T for
musrfit classes), and the addRule() helper to AddRule(). The Qt
override highlightBlock() keeps its lowercase name as required by
QSyntaxHighlighter. Doc comments updated to reference PMsrHighlighter.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uncaught std::bad_any_cast calls terminated the program when an HDF4
attribute's stored type didn't match the expected type (e.g. vector<int>
instead of int, or a non-string units attribute). Wrap all eight attribute
casts with try/catch so type mismatches produce a warning rather than a crash.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix missing comma that concatenated "ab" and "skg" into the
bogus keyword "abskg", dropping abragam/skewedGss highlighting.
- Remove duplicate "snkzf" short name in the theory keyword list.
- Add missing theory keywords: "sg" (simpleGss short form),
"userFcn" and its short form "u", to match src/include/PTheory.h.
- Use std::as_const in highlightBlock's range-for to avoid a
potential QVector detach on every call.
- Pass QTextCharFormat to addRule by const reference.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
macOS 26 (Darwin 25.5.0) introduced stricter Clang module map rules in
DarwinFoundation1.modulemap: the '_c_standard_library_obsolete' module
now requires the 'found_incompatible_headers__check_search_paths' feature,
which CLING's runtime interpreter does not set. When fftw3.h (from
/opt/homebrew/include) was parsed by CLING via the inlined PStartupHandler
dictionary payload, it triggered this module map error, causing all
PStartupHandler signal/slot connections to fail.
Two fixes:
- PMusr.h: guard '#include "fftw3.h"' with '#ifndef __CLING__'. No fftw
types appear in PMusr.h class definitions, so CLING does not need it
for reflection. This is the primary runtime fix.
- CMakeLists.txt (src/classes and all src/external libs): replace
'-I${FFTW3_INCLUDE}', '-I${Boost_INCLUDE_DIRS}', '-I${GSL_INCLUDE_DIRS}',
and '-I${ROOT_INCLUDE_DIRS}' with '-isystem' in all root_generate_dictionary
OPTIONS blocks, and change 'include_directories(${FFTW3_INCLUDE})' to
'include_directories(SYSTEM ...)'. Internal project paths (MUSRFIT_INC,
BMW_TOOLS_INC, NONLOCAL_INC, CMAKE_CURRENT_SOURCE_DIR, etc.) keep '-I'.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit introduces a syntax/keyword highlighting mechanism for musredit
(Qt6 version only). This is done by way of a custom "highligher" class
derived from QSyntaxHighlighter, which allows for fonts, weights, and
colours, to be applied to specific text components via regular expression
rules.
The colour scheme provided is experimental/arbitrary; it uses the "Tableau 20"
palette, which provides reasonable contrast.
Convert all TString::Tokenize() call sites to PStringUtils::Split. The
persisted RunSummary TObjArray (read from the MusrRoot file via
FindObjectAny and iterated with TObjArrayIter) and the run-title TObjString
returned by TLemRunHeader::GetRunTitle() remain ROOT types, since they come
from external interfaces / on-disk format, not from tokenization.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace ROOT's TString::Tokenize() + TObjArray/TObjString token
handling with the dependency-free PStringUtils::Split() across all
command/theory parsing in PFitter (GetPhaseParams, GetParFromFun,
CheckCommands and the Execute* helpers). Split() mirrors Tokenize()
semantics (delimiter-set, skips empty tokens), so token counts and
indices are unchanged. Each token is still copied into a TString,
so the downstream Atoi/Atof/IsFloat/IsDigit/Contains/CompareTo logic
stays as-is.
Using a std::vector<std::string> removes the manual TObjArray
cleanup and incidentally fixes three pre-existing leaks: the
tokens array in ExecuteFitRange was never freed, and the early
return paths in ExecutePrintLevel and the SECTOR check in
CheckCommands skipped the cleanup.
Build of libPMusr and musrfit is clean; full ctest suite passes
(85/85).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace ROOT's TString::Tokenize() + TObjArray/TObjString token
handling with the dependency-free PStringUtils::Split() across
msr2msr_run, msr2msr_param and msr2msr_theory. Split() mirrors
Tokenize() semantics (skips empty tokens), so token counts and
indices are unchanged.
Using a std::vector<std::string> removes the manual TObjArray
cleanup and incidentally fixes a pre-existing leak in
msr2msr_theory, which tokenized in every branch but never deleted
the TObjArray.
Since msr2msr links only against ROOT (not PMusr, where
PStringUtils lives) and PStringUtils is pure C++17, compile
classes/PStringUtils.cpp directly into the msr2msr executable
rather than pulling in the whole PMusr library.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GCC's -Wstringop-overflow flagged the memcpy in SobolIni() with a bound
of (size_t)(-4): on the (in practice unreachable) path where the Sobol
generator polynomial 'powers' is 0, the bit-count loop leaves inibits at
its initial -1, so inibits*sizeof underflows. The generator-polynomial
table always has a non-zero first column, so this never happens at run
time, but the compiler cannot prove it.
Guard the copy with 'if (inibits > 0)', which silences the false-positive
warning and hardens the edge case without changing behaviour for valid
input.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
OpenMP 5.2 deprecates the '-' reduction operator (-Wdeprecated-openmp)
because it is functionally identical to '+': the private reduction copy
is initialised to 0 and partial results are combined by addition in both
cases. All affected loops accumulate with 'mllh += ...', so switching to
reduction(+:mllh) is results-identical and silences the warning.
Fixes the pragma in PRunMuMinus.cpp and the two in PRunSingleHisto.cpp,
and corrects the now-inaccurate "for subtraction" doc comment.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The QProcess::finished lambdas used [=], which implicitly captures 'this'
to call member functions (exitStatusMusrWiz, fileReload,
exitStatusMusrSetSteps). Implicit 'this' capture via [=] is deprecated in
C++20; name it explicitly with [=, this]. Applied to both the qt6 and qt5
copies.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The fallback date placeholder string contained the sequence '??-', which
the compiler interprets as a trigraph for '~' (-Wtrigraphs warning).
Escape the question marks (\?) so the literal string is unchanged while
the warning is silenced.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the ROOT TString::Tokenize()/TObjArray/TObjString machinery used
for parsing the RGB color code in OnCharacters() with the C++17
PStringUtils helpers (Split/IsInt/ToInt). Drops the manual heap cleanup
and the <TObjArray.h>/<TObjString.h> includes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
IsInt now recognises (possibly signed) integers such as "-5" or "+42",
making it slightly more permissive than TString::IsDigit(). A lone sign,
a double sign, or a sign following a digit are still rejected. strToNum
test expectations updated accordingly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stand-alone test under src/tests/strToNum exercising every PStringUtils
method (Split / IsInt / IsFloat / ToInt / ToDouble / IsEqualNoCase /
ContainsNoCase / BeginsWithNoCase). Three modes: built-in pass/fail
suite, ad-hoc inspection of command-line strings, and an interactive
prompt. Adds -h/--help and -i/--interactive options. Pure C++17, no
ROOT dependency.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ToDouble() had the same shortcoming as ToInt(): wrapping strtod() with a
nullptr endptr made a failed conversion indistinguishable from a
legitimate 0.0. Add an optional `bool *ok` out-parameter that reports
success. strtod() (with endptr + errno) is kept instead of
std::from_chars so the accepted input set stays identical to
TString::Atof() (leading whitespace skipped, leading '+' honoured,
trailing characters ignored); ok is set false on a non-numeric string or
an ERANGE overflow. The parameter defaults to nullptr, so existing call
sites keep compiling unchanged.
Convert the IsFloat-guarded ToDouble call sites in PMsrHandler to the
single-parse ToDouble(token, &ok) form (replacing the IsFloat() guard +
separate ToDouble() that parsed every token twice). All downstream
>=0 / <=0 / range checks are preserved, and push_back sites only append
on success so no spurious 0.0 is stored on error. Number-vs-keyword
discriminators (pos.error/boundary "none", rrf_phase/fourier-phase parX)
are restructured so the keyword branch is taken when ok is false.
As a side effect this fixes a latent gap in the GLOBAL rrf_freq handler,
where a non-numeric frequency previously slipped through with a stale
value instead of raising an error.
The IsFloat-guarded ToInt fit-range offsets (fgb/lgb) are intentionally
left untouched, as there the guard type differs from the conversion.
All 85 integration tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ToInt() previously wrapped strtol() with a nullptr endptr, so a failed
conversion was indistinguishable from a legitimate 0 (matching the old
TString::Atoi() behaviour). Switch the implementation to std::from_chars
and add an optional `bool *ok` out-parameter that reports success: it is
set to false on a non-numeric string or an out-of-range value, true
otherwise. Leading whitespace is skipped and trailing characters are
ignored, preserving the Atoi-like prefix semantics. The parameter
defaults to nullptr, so existing call sites keep compiling unchanged.
Convert the parse-validation call sites in PMsrHandler to the single
-parse ToInt(token, &ok) form, replacing the IsInt() guard + separate
ToInt() (which parsed every token twice). All downstream >0 / >=0 /
range / enum checks are preserved.
Left untouched the call sites where IsInt() acts as a structural
discriminator rather than a numeric validator (write path, xy-data
index-vs-label, fParamInUse usage scans) and the IsFloat-guarded ToInt
offsets, where switching to ToInt(&ok) would change parsing semantics.
All 85 integration tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reduce the ROOT footprint of the MSR parser by removing the pervasive
TString::Tokenize / TObjArray / TObjString / dynamic_cast pattern (28
tokenize sites, 14 TObjArray, 106 TObjString) used to split lines into
tokens, together with the manual `delete tokens` cleanup.
Add a new dependency-free C++17 utility class PStringUtils (Split, IsInt,
IsFloat, ToInt, ToDouble, IsEqualNoCase, ContainsNoCase, BeginsWithNoCase)
that replicates the relevant TString semantics exactly, so it can be reused
elsewhere in the suite. IsInt/IsFloat tolerate surrounding whitespace to
match TString::IsDigit/IsFloat (needed for tokens split on ',' / ';' only).
The public API and the PMusr.h data structures keep TString unchanged; only
the internal tokenizing logic is rewritten. Net -451 lines in
PMsrHandler.cpp. All 85 integration tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
getValues()/getErrors() return a std::vector by value, but the QVector was
built from begin() of one call and end() of a second call -> iterators into
two distinct temporaries -> UB and a segfault in getValues(). Bind the result
to a single local before taking begin()/end(). Same in getPosErr/getNegErr.
This surfaced reliably when plotting a <python>-generated variable added via
the GUI (the script path was unaffected as it reads getValues() only once).
Also fix check() to validate the actually selected collections
(GetCollection(idx[i]) instead of GetCollection(i)).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Use the new coll[]/collErr[] interface so sigmaSC-vs-temp-py.txt computes both
SigmaSC_10 and SigmaSC_150 in one block instead of one block per collection.
Collection 0 is addressed by index (coll[0]) and collection 1 by name
(coll['...Tscan.db']) to show both addressing modes. Output is unchanged.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A <python> variable block previously saw only the parameters of the single
collection the variable is linked to via 'col', injected as bare names plus
par[]/parErr[]. To compute variables for several collections one therefore
needed a separate block per collection.
Now every loaded collection is additionally injected as coll[]/collErr[]
dictionaries, addressable both by integer index (matching 'col <idx>' /
'select <idx>') and by collection name; index and name keys reference the same
per-parameter dict. A single block placed after all 'var = python' declarations
can thus compute - and combine - variables for any collection, e.g.
coll[0]['Sigma'] or coll['NAME.db']['Sigma'].
The bound-collection bare names / par[] / parErr[] are unchanged, so existing
scripts and GUI variables keep working. The collection list is threaded through
PmuppScript::var_cmd and the GUI add()/check() paths into PVarHandler.
Verified: a one-block script using both an index key and a name key reproduces
the numeric output of the equivalent two-block script bit-for-bit.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
suter_aandClaude Sonnet 5
ext-mcfadd_r