rugnux: validate offline spot-finding settings and bound numeric options
The offline rugnux path never ran CheckDataProcessingSettings (only the online broker/receiver paths did), so the new --min-pix-per-spot and --spot-low-resolution knobs were unbounded: --min-pix-per-spot 0 silently disabled the per-spot filter, and a low-resolution limit finer than the high-resolution limit made spot finding reject every pixel with no diagnostic. Add a parse_number_arg<T> helper (integral or floating, with optional inclusive bounds) that rejects non-numeric input, trailing garbage, and out-of-range values, replacing the raw atoi/atof in the spot-finding options; parse_double_arg/parse_float_arg become thin wrappers over it. After assembling the settings, validate them with the same CheckDataProcessingSettings the online receivers use, which also enforces the cross-field low>=high-resolution constraint. Also scrubs a residual sample-name comment (the r=6 box-default note) in this file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+59
-19
@@ -8,9 +8,11 @@
|
||||
#include <csignal>
|
||||
#include <getopt.h>
|
||||
#include <iostream>
|
||||
#include <limits>
|
||||
#include <sstream>
|
||||
#include <string>
|
||||
#include <thread>
|
||||
#include <type_traits>
|
||||
#include <vector>
|
||||
|
||||
#include "../reader/JFJochHDF5Reader.h"
|
||||
@@ -279,28 +281,54 @@ bool parse_float_strict(const std::string &t, float &out) {
|
||||
}
|
||||
};
|
||||
|
||||
// Parse a required numeric option argument, or print a clear error and exit. getopt hands option
|
||||
// arguments over as raw C strings; atof() silently returns 0 on non-numeric input (so a typo like
|
||||
// "--beam-x 21OO" would move the beam to pixel 0) and std::stod() throws std::invalid_argument,
|
||||
// which terminates the program. This rejects both, plus trailing garbage like "1.5foo".
|
||||
double parse_double_arg(const char *arg, const char *option_name, Logger &logger) {
|
||||
// Parse a required numeric option argument, optionally bounded to [min_value, max_value], or print a
|
||||
// clear error and exit. getopt hands option arguments over as raw C strings; atoi()/atof() silently
|
||||
// return 0 on non-numeric input (so a typo like "--min-pix-per-spot 2O" becomes 2 or 0) and std::sto*
|
||||
// throws, which would terminate the program. This rejects non-numeric input, trailing garbage
|
||||
// ("1.5foo"), integer overflow, and out-of-range values. T may be integral or floating-point; the
|
||||
// bounds default to the full representable range (i.e. unbounded).
|
||||
template <typename T>
|
||||
T parse_number_arg(const char *arg, const char *option_name, Logger &logger,
|
||||
T min_value = std::numeric_limits<T>::lowest(),
|
||||
T max_value = std::numeric_limits<T>::max()) {
|
||||
std::string s = arg ? arg : "";
|
||||
trim_in_place(s);
|
||||
T value{};
|
||||
bool parsed = false;
|
||||
if (!s.empty()) {
|
||||
try {
|
||||
size_t idx = 0;
|
||||
double out = std::stod(s, &idx);
|
||||
if (idx == s.size())
|
||||
return out;
|
||||
if constexpr (std::is_integral_v<T>) {
|
||||
const long long v = std::stoll(s, &idx);
|
||||
value = static_cast<T>(v);
|
||||
parsed = (idx == s.size()) && (static_cast<long long>(value) == v); // no overflow
|
||||
} else {
|
||||
value = static_cast<T>(std::stod(s, &idx));
|
||||
parsed = (idx == s.size());
|
||||
}
|
||||
} catch (...) {}
|
||||
}
|
||||
logger.Error("Invalid numeric value for {}: '{}'", option_name, arg ? arg : "<null>");
|
||||
print_usage();
|
||||
exit(EXIT_FAILURE);
|
||||
if (!parsed) {
|
||||
logger.Error("Invalid numeric value for {}: '{}'", option_name, arg ? arg : "<null>");
|
||||
print_usage();
|
||||
exit(EXIT_FAILURE);
|
||||
}
|
||||
if (value < min_value || value > max_value) {
|
||||
logger.Error("Value for {} out of range: {} (expected {} to {})",
|
||||
option_name, value, min_value, max_value);
|
||||
print_usage();
|
||||
exit(EXIT_FAILURE);
|
||||
}
|
||||
return value;
|
||||
}
|
||||
|
||||
// Thin unbounded wrappers for the existing floating-point call sites.
|
||||
double parse_double_arg(const char *arg, const char *option_name, Logger &logger) {
|
||||
return parse_number_arg<double>(arg, option_name, logger);
|
||||
}
|
||||
|
||||
float parse_float_arg(const char *arg, const char *option_name, Logger &logger) {
|
||||
return static_cast<float>(parse_double_arg(arg, option_name, logger));
|
||||
return parse_number_arg<float>(arg, option_name, logger);
|
||||
}
|
||||
|
||||
bool parse_on_off(const char *arg, bool &out) {
|
||||
@@ -687,28 +715,28 @@ int main(int argc, char **argv) {
|
||||
break;
|
||||
}
|
||||
case OPT_SPOT_SIGMA:
|
||||
sigma_spot_finding = atof(optarg);
|
||||
sigma_spot_finding = parse_number_arg<float>(optarg, "--spot-sigma", logger, 1.0f);
|
||||
logger.Info("Noise threshold level for spot finding set to {:.2f} sigma", sigma_spot_finding);
|
||||
break;
|
||||
case OPT_SPOT_THRESHOLD:
|
||||
photon_count_threshold_spot_finding = atoi(optarg);
|
||||
photon_count_threshold_spot_finding = parse_number_arg<int64_t>(optarg, "--spot-threshold", logger, 0);
|
||||
logger.Info("Photon-count threshold level for spot finding set to {:d}",
|
||||
photon_count_threshold_spot_finding);
|
||||
break;
|
||||
case OPT_MIN_PIX_PER_SPOT:
|
||||
min_pix_per_spot = atoi(optarg);
|
||||
min_pix_per_spot = parse_number_arg<int64_t>(optarg, "--min-pix-per-spot", logger, 1);
|
||||
logger.Info("Minimum pixels per spot set to {:d}", min_pix_per_spot);
|
||||
break;
|
||||
case OPT_SPOT_LOW_RESOLUTION:
|
||||
d_max_spot_finding = atof(optarg);
|
||||
d_max_spot_finding = parse_number_arg<float>(optarg, "--spot-low-resolution", logger, 0.0f);
|
||||
logger.Info("Low resolution limit for spot finding set to {:.1f} A", d_max_spot_finding);
|
||||
break;
|
||||
case OPT_SPOT_RESOLUTION:
|
||||
d_min_spot_finding = atof(optarg);
|
||||
d_min_spot_finding = parse_number_arg<float>(optarg, "--spot-high-resolution", logger, 0.0f);
|
||||
logger.Info("High resolution limit for spot finding set to {:.2f} A", d_min_spot_finding);
|
||||
break;
|
||||
case OPT_MAX_SPOTS:
|
||||
max_spot_count_override = atoll(optarg);
|
||||
max_spot_count_override = parse_number_arg<int64_t>(optarg, "--max-spots", logger, 1);
|
||||
break;
|
||||
case OPT_AZINT_ONLY:
|
||||
azint_only = true;
|
||||
@@ -1375,7 +1403,7 @@ int main(int argc, char **argv) {
|
||||
// wider on the detector than the r1=4 monochromatic-rotation default assumes. A larger signal
|
||||
// box lets the profile-fit integrator capture the whole spot while its profile weighting keeps
|
||||
// the extra background from adding noise (a plain box-sum degrades with it). Measured R-free
|
||||
// gains on serial stills (OCP, LOV). An explicit --integration-radius always wins.
|
||||
// gains on serial stills. An explicit --integration-radius always wins.
|
||||
BraggIntegrationSettings bis = experiment.GetBraggIntegrationSettings();
|
||||
bis.R1(6.0f).R2(8.0f).R3(12.0f);
|
||||
experiment.ImportBraggIntegrationSettings(bis);
|
||||
@@ -1410,6 +1438,18 @@ int main(int argc, char **argv) {
|
||||
if (d_max_spot_finding > 0.0f)
|
||||
spot_settings.low_resolution_limit = d_max_spot_finding;
|
||||
|
||||
// Validate the assembled spot-finding settings the same way the online receivers do (broker and
|
||||
// receiver call this same function). It enforces the cross-field constraints that per-argument
|
||||
// bounds cannot express - in particular that the low-resolution limit is coarser than the
|
||||
// high-resolution limit, so --spot-low-resolution below the high-res cut no longer silently
|
||||
// rejects every pixel.
|
||||
try {
|
||||
DiffractionExperiment::CheckDataProcessingSettings(spot_settings);
|
||||
} catch (const std::exception &e) {
|
||||
logger.Error("Invalid spot-finding settings: {}", e.what());
|
||||
return 1;
|
||||
}
|
||||
|
||||
// Run the shared full-analysis workflow (rotation indexing + scaling/merging live in
|
||||
// Rugnux; the experiment above carries all algorithm settings).
|
||||
ProcessConfig config;
|
||||
|
||||
Reference in New Issue
Block a user