common: take ROIAzimuthal phi as plain floats, not optionals
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 10m48s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 11m35s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 12m2s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 12m58s
Build Packages / build:rpm (rocky8) (push) Successful in 13m0s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 13m58s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 14m29s
Build Packages / XDS test (durin plugin) (push) Successful in 7m30s
Build Packages / Generate python client (push) Successful in 39s
Build Packages / Build documentation (push) Successful in 51s
Build Packages / Create release (push) Skipped
Build Packages / build:rpm (ubuntu2404) (push) Successful in 10m11s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 10m55s
Build Packages / build:rpm (rocky9) (push) Successful in 12m15s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 9m11s
Build Packages / XDS test (neggia plugin) (push) Successful in 8m33s
Build Packages / DIALS test (push) Successful in 12m0s
Build Packages / Unit tests (push) Successful in 57m36s

The phi bounds were stored as plain floats internally but passed through
std::optional in the constructor, which obscured the intent. Take them
as float arguments defaulting to 0 instead. The convention is explicit:
phi_min == phi_max means the full ring (all angles), a sector wraps
across 0 when phi_min > phi_max, and otherwise phi_min <= phi_max. The
"both bounds or none" rule belongs at the API/file boundary
(OpenAPIConvert, reader), not in the core class, so it lives there now.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-19 11:03:40 +02:00
parent 6b95600260
commit eadbce51bd
5 changed files with 28 additions and 31 deletions
+4 -3
View File
@@ -462,11 +462,12 @@ ROIDefinition Convert(const org::openapitools::server::model::Roi_definitions& i
for (const auto &i: input.getCircle().getRois())
output.circles.emplace_back(ROICircle(i.getName(), i.getCenterXPxl(), i.getCenterYPxl(), i.getRadiusPxl()));
for (const auto &i: input.getAzim().getRois()) {
std::optional<float> phi_min, phi_max;
if (i.phiMinDegIsSet())
// A sector needs both bounds; if only one is given, treat it as a full ring.
float phi_min = 0, phi_max = 0;
if (i.phiMinDegIsSet() && i.phiMaxDegIsSet()) {
phi_min = i.getPhiMinDeg();
if (i.phiMaxDegIsSet())
phi_max = i.getPhiMaxDeg();
}
output.azimuthal.emplace_back(ROIAzimuthal(i.getName(),
(i.getQMaxRecipA() == 0.0) ? 0.0 : 2.0f * PI / i.getQMaxRecipA(),
(i.getQMinRecipA() == 0.0) ? 0.0 : 2.0f * PI / i.getQMinRecipA(),
+9 -16
View File
@@ -14,8 +14,10 @@ static float NormalizePhi_deg(float phi) {
}
ROIAzimuthal::ROIAzimuthal(const std::string &in_name, float in_d_min_A, float in_d_max_A,
std::optional<float> in_phi_min_deg, std::optional<float> in_phi_max_deg)
: ROIElement(in_name) {
float in_phi_min_deg, float in_phi_max_deg)
: ROIElement(in_name),
phi_min_deg(NormalizePhi_deg(in_phi_min_deg)),
phi_max_deg(NormalizePhi_deg(in_phi_max_deg)) {
if ((in_d_min_A <= 0) || (in_d_max_A <= 0))
throw JFJochException(JFJochExceptionCategory::InputParameterInvalid,
"Resolution cannot be zero or negative");
@@ -26,12 +28,6 @@ ROIAzimuthal::ROIAzimuthal(const std::string &in_name, float in_d_min_A, float i
d_max_A = in_d_max_A;
d_min_A = in_d_min_A;
}
if (in_phi_min_deg && in_phi_max_deg) {
has_phi = true;
phi_min_deg = NormalizePhi_deg(in_phi_min_deg.value());
phi_max_deg = NormalizePhi_deg(in_phi_max_deg.value());
}
}
float ROIAzimuthal::GetDMin_A() const {
@@ -43,7 +39,7 @@ float ROIAzimuthal::GetDMax_A() const {
}
bool ROIAzimuthal::HasPhi() const {
return has_phi;
return phi_min_deg != phi_max_deg;
}
float ROIAzimuthal::GetPhiMin_deg() const {
@@ -57,13 +53,11 @@ float ROIAzimuthal::GetPhiMax_deg() const {
bool ROIAzimuthal::CheckROI(int64_t x, int64_t y, float resolution, float phi_deg) const {
if (resolution < d_min_A || resolution > d_max_A)
return false;
if (!has_phi)
return true;
// A sector with phi_min > phi_max wraps across 0 degrees.
if (phi_min_deg == phi_max_deg)
return true; // full ring: all angles
if (phi_min_deg <= phi_max_deg)
return (phi_deg >= phi_min_deg) && (phi_deg <= phi_max_deg);
else
return (phi_deg >= phi_min_deg) || (phi_deg <= phi_max_deg);
return (phi_deg >= phi_min_deg) || (phi_deg <= phi_max_deg); // sector wraps across 0
}
float ROIAzimuthal::GetQMax_recipA() const {
@@ -81,7 +75,6 @@ ROIConfig ROIAzimuthal::ExportMetadata() const {
.type = ROIConfig::ROIType::Azim,
.name = name,
.azim = ROIConfigAzim{.qmin = qmin, .qmax = qmax,
.phi_min = has_phi ? phi_min_deg : 0.0f,
.phi_max = has_phi ? phi_max_deg : 0.0f}
.phi_min = phi_min_deg, .phi_max = phi_max_deg}
};
}
+5 -6
View File
@@ -3,24 +3,23 @@
#pragma once
#include <optional>
#include "ROIElement.h"
class ROIAzimuthal : public ROIElement {
float d_min_A, d_max_A;
bool has_phi = false;
float phi_min_deg = 0; // normalized to [0, 360), used only when has_phi
// Azimuthal-angle sector, normalized to [0, 360). phi_min_deg == phi_max_deg means
// the full ring (all angles); a sector with phi_min_deg > phi_max_deg wraps across 0.
float phi_min_deg = 0;
float phi_max_deg = 0;
public:
ROIAzimuthal(const std::string &name, float d_min_A, float d_max_A,
std::optional<float> phi_min_deg = std::nullopt,
std::optional<float> phi_max_deg = std::nullopt);
float phi_min_deg = 0, float phi_max_deg = 0);
~ROIAzimuthal() override = default;
[[nodiscard]] float GetDMin_A() const;
[[nodiscard]] float GetDMax_A() const;
[[nodiscard]] float GetQMin_recipA() const;
[[nodiscard]] float GetQMax_recipA() const;
[[nodiscard]] bool HasPhi() const;
[[nodiscard]] bool HasPhi() const; // a sector is set (phi_min_deg != phi_max_deg)
[[nodiscard]] float GetPhiMin_deg() const;
[[nodiscard]] float GetPhiMax_deg() const;
+5 -3
View File
@@ -274,9 +274,11 @@ void JFJochHDF5Reader::ReadROIMetadata(HDF5ReadOnlyFile &file, JFJochReaderDatas
else if (type == "azim") {
const float qmin = file.GetFloat(base + "/q_min_recipA");
const float qmax = file.GetFloat(base + "/q_max_recipA");
std::optional<float> phi_min, phi_max;
if (file.Exists(base + "/phi_min_deg")) phi_min = file.GetFloat(base + "/phi_min_deg");
if (file.Exists(base + "/phi_max_deg")) phi_max = file.GetFloat(base + "/phi_max_deg");
float phi_min = 0, phi_max = 0;
if (file.Exists(base + "/phi_min_deg") && file.Exists(base + "/phi_max_deg")) {
phi_min = file.GetFloat(base + "/phi_min_deg");
phi_max = file.GetFloat(base + "/phi_max_deg");
}
const float d_min = (qmax == 0.0f) ? 0.0f : 2.0f * static_cast<float>(PI) / qmax;
const float d_max = (qmin == 0.0f) ? 0.0f : 2.0f * static_cast<float>(PI) / qmin;
defs.azimuthal.emplace_back(name, d_min, d_max, phi_min, phi_max);
+5 -3
View File
@@ -226,9 +226,11 @@ TEST_CASE("ROIAzimuthal_Phi_Normalize", "[ROIMap]") {
REQUIRE(sector.GetPhiMin_deg() == 330.0f);
REQUIRE(sector.GetPhiMax_deg() == 40.0f);
// Only one phi bound provided -> treated as full ring
ROIAzimuthal one_bound("roi2", 2.0, 4.0, 30.0f);
REQUIRE_FALSE(one_bound.HasPhi());
// Default (no phi bounds) and equal bounds are both the full ring
ROIAzimuthal full("roi2", 2.0, 4.0);
REQUIRE_FALSE(full.HasPhi());
ROIAzimuthal equal_bounds("roi3", 2.0, 4.0, 45.0f, 45.0f);
REQUIRE_FALSE(equal_bounds.HasPhi());
}
TEST_CASE("ROIAzimuthal_Phi_MarkROI", "[ROIMap]") {