From bf69037a74fb7cf2c759593eea52b551d180e100 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Thu, 3 Jul 2025 09:06:19 +0100 Subject: [PATCH 01/23] Split off some calculations from PanelsSurface By moving as much maths as possible to a separate class, we can then use this class in the new Instrument View window to avoid code duplication. Added some tests as well. --- Framework/API/CMakeLists.txt | 3 + .../inc/MantidAPI/PanelsSurfaceCalculator.h | 37 +++ Framework/API/src/PanelsSurfaceCalculator.cpp | 216 +++++++++++++++++ .../API/test/PanelsSurfaceCalculatorTest.h | 116 +++++++++ .../PythonInterface/mantid/api/CMakeLists.txt | 1 + .../src/Exports/PanelsSurfaceCalculator.cpp | 17 ++ .../InstrumentView/PanelsSurface.h | 5 +- .../instrumentview/src/PanelsSurface.cpp | 223 ++---------------- .../test/InstrumentWidget/PanelsSurfaceTest.h | 5 +- 9 files changed, 414 insertions(+), 209 deletions(-) create mode 100644 Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h create mode 100644 Framework/API/src/PanelsSurfaceCalculator.cpp create mode 100644 Framework/API/test/PanelsSurfaceCalculatorTest.h create mode 100644 Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp diff --git a/Framework/API/CMakeLists.txt b/Framework/API/CMakeLists.txt index 74bdb83eeaeb..a71978caac32 100644 --- a/Framework/API/CMakeLists.txt +++ b/Framework/API/CMakeLists.txt @@ -120,6 +120,7 @@ set(SRC_FILES src/NumericAxis.cpp src/NumericAxisValidator.cpp src/OrientedLatticeValidator.cpp + src/PanelsSurfaceCalculator.cpp src/ParamFunction.cpp src/ParameterReference.cpp src/ParameterTie.cpp @@ -318,6 +319,7 @@ set(INC_FILES inc/MantidAPI/NumericAxis.h inc/MantidAPI/NumericAxisValidator.h inc/MantidAPI/OrientedLatticeValidator.h + inc/MantidAPI/PanelsSurfaceCalculator.h inc/MantidAPI/ParamFunction.h inc/MantidAPI/ParameterReference.h inc/MantidAPI/ParameterTie.h @@ -452,6 +454,7 @@ set(TEST_FILES NumericAxisTest.h NumericAxisValidatorTest.h OrientedLatticeValidatorTest.h + PanelsSurfaceCalculatorTest.h ParamFunctionAttributeHolderTest.h ParameterReferenceTest.h ParameterTieTest.h diff --git a/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h new file mode 100644 index 000000000000..e93d413b8619 --- /dev/null +++ b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h @@ -0,0 +1,37 @@ +// Mantid Repository : https://github.com/mantidproject/mantid +// +// Copyright © 2025 ISIS Rutherford Appleton Laboratory UKRI, +// NScD Oak Ridge National Laboratory, European Spallation Source, +// Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS +// SPDX - License - Identifier: GPL - 3.0 + +#pragma once + +#include "MantidAPI/DllConfig.h" +#include "MantidGeometry/Instrument/ComponentInfo.h" +#include "MantidKernel/Logger.h" +#include "MantidKernel/Quat.h" +#include "MantidKernel/V3D.h" + +using Mantid::Geometry::ComponentInfo; +using Mantid::Kernel::V3D; + +namespace Mantid { +namespace API { +class MANTID_API_DLL PanelsSurfaceCalculator { +public: + void setupBasisAxes(const V3D &zaxis, V3D &xaxis, V3D &yaxis) const; + std::vector retrievePanelCorners(const ComponentInfo &componentInfo, const size_t rootIndex) const; + V3D calculatePanelNormal(const std::vector &panelCorners) const; + bool isBankFlat(const ComponentInfo &componentInfo, size_t bankIndex, const std::vector &tubes, + const V3D &normal); + V3D calculateBankNormal(const ComponentInfo &componentInfo, const std::vector &tubes); + void setBankVisited(const ComponentInfo &componentInfo, size_t bankIndex, std::vector &visitedComponents) const; + size_t findNumDetectors(const ComponentInfo &componentInfo, const std::vector &components) const; + Mantid::Kernel::Quat calcBankRotation(const V3D &detPos, V3D normal, const V3D &zAxis, const V3D &yAxis, + const V3D &samplePosition) const; + +private: + Mantid::Kernel::Logger g_log = Mantid::Kernel::Logger("PanelsSurfaceCalculator"); +}; +} // namespace API +} // namespace Mantid diff --git a/Framework/API/src/PanelsSurfaceCalculator.cpp b/Framework/API/src/PanelsSurfaceCalculator.cpp new file mode 100644 index 000000000000..197c4d43cae4 --- /dev/null +++ b/Framework/API/src/PanelsSurfaceCalculator.cpp @@ -0,0 +1,216 @@ +// Mantid Repository : https://github.com/mantidproject/mantid +// +// Copyright © 2025 ISIS Rutherford Appleton Laboratory UKRI, +// NScD Oak Ridge National Laboratory, European Spallation Source, +// Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS +// SPDX - License - Identifier: GPL - 3.0 + +#pragma once + +#include "MantidAPI/PanelsSurfaceCalculator.h" +#include "MantidAPI/MatrixWorkspace.h" +#include "MantidGeometry/Objects/IObject.h" +#include "MantidGeometry/Rendering/GeometryHandler.h" +#include "MantidGeometry/Rendering/ShapeInfo.h" + +using namespace Mantid::Geometry; +using Mantid::Beamline::ComponentType; +using Mantid::Kernel::V3D; + +namespace Mantid::API { +/** + * Given the z axis, define the x and y ones. + * @param zaxis :: A given vector in 3d space to become the z axis of a + * coordinate system. + * @param xaxis :: An output arbitrary vector perpendicular to zaxis. + * @param yaxis :: An output arbitrary vector perpendicular to both zaxis and + * xaxis. + */ +void PanelsSurfaceCalculator::setupBasisAxes(const Mantid::Kernel::V3D &zaxis, Mantid::Kernel::V3D &xaxis, + Mantid::Kernel::V3D &yaxis) const { + double R, theta, phi; + zaxis.getSpherical(R, theta, phi); + if (theta <= 45.0) { + xaxis = Mantid::Kernel::V3D(1, 0, 0); + } else if (phi <= 45.0) { + xaxis = Mantid::Kernel::V3D(0, 1, 0); + } else { + xaxis = Mantid::Kernel::V3D(0, 0, 1); + } + yaxis = zaxis.cross_prod(xaxis); + yaxis.normalize(); + xaxis = yaxis.cross_prod(zaxis); +} + +std::vector +PanelsSurfaceCalculator::retrievePanelCorners(const Mantid::Geometry::ComponentInfo &componentInfo, + const size_t rootIndex) const { + auto panel = componentInfo.quadrilateralComponent(rootIndex); + auto child = rootIndex; + + // Find shapeInfo for one of the children + do { + const auto &children = componentInfo.children(child); + child = children[0]; + } while (!componentInfo.isDetector(child)); + + const auto &shape = componentInfo.shape(child); + const auto &shapeInfo = shape.getGeometryHandler()->shapeInfo(); + const auto &points = shapeInfo.points(); + double xoff = 0.0; + double yoff = 0.0; + + // Find x and y widths of detectors and treat as offsets; + for (size_t i = 0; i < points.size() - 1; ++i) { + double xdiff = std::abs(points[i + 1].X() - points[i].X()); + double ydiff = std::abs(points[i + 1].Y() - points[i].Y()); + if (xdiff != 0) + xoff = xdiff * 0.5; + if (ydiff != 0) + yoff = ydiff * 0.5; + } + + std::vector corners{ + componentInfo.position(panel.bottomLeft), componentInfo.position(panel.bottomRight), + componentInfo.position(panel.topRight), componentInfo.position(panel.topLeft)}; + + // Find xmin, xmax, ymin and ymax + double xmin = corners[0].X(); + double xmax = corners[0].X(); + double ymin = corners[0].Y(); + double ymax = corners[0].Y(); + + for (const auto &corner : corners) { + xmin = corner.X() < xmin ? corner.X() : xmin; + xmax = corner.X() > xmax ? corner.X() : xmax; + ymin = corner.Y() < ymin ? corner.Y() : ymin; + ymax = corner.Y() > ymax ? corner.Y() : ymax; + } + + // apply offsets + for (auto &corner : corners) { + auto x = corner.X(); + auto y = corner.Y(); + corner.setX(x == xmin ? x - xoff : x + xoff); + corner.setY(y == ymin ? y - yoff : y + yoff); + } + + return corners; +} + +Mantid::Kernel::V3D +PanelsSurfaceCalculator::calculatePanelNormal(const std::vector &panelCorners) const { + // find the normal + auto xaxis = panelCorners[1] - panelCorners[0]; + auto yaxis = panelCorners[3] - panelCorners[0]; + const auto normal = normalize(xaxis.cross_prod(yaxis)); + return normal; +} + +bool PanelsSurfaceCalculator::isBankFlat(const ComponentInfo &componentInfo, size_t bankIndex, + const std::vector &tubes, const Mantid::Kernel::V3D &normal) { + for (auto tube : tubes) { + const auto &children = componentInfo.children(tube); + const auto vector = normalize(componentInfo.position(children[0]) - componentInfo.position(children[1])); + if (fabs(vector.scalar_prod(normal)) > Mantid::Kernel::Tolerance) { + this->g_log.warning() << "Assembly " << componentInfo.name(bankIndex) << " isn't flat.\n"; + return false; + } + } + return true; +} + +Mantid::Kernel::V3D PanelsSurfaceCalculator::calculateBankNormal(const ComponentInfo &componentInfo, + const std::vector &tubes) { + // calculate normal from first two tubes in bank as before + const auto &tube0 = componentInfo.children(tubes[0]); + const auto &tube1 = componentInfo.children(tubes[1]); + auto pos = componentInfo.position(tube0[0]); + auto x = componentInfo.position(tube0[1]) - pos; + x.normalize(); + + auto y = componentInfo.position(tube1[0]) - pos; + y.normalize(); + auto normal = x.cross_prod(y); + + if (normal.nullVector()) { + y = componentInfo.position(tube1[1]) - componentInfo.position(tube1[0]); + y.normalize(); + normal = x.cross_prod(y); + } + + normal.normalize(); + + if (normal.nullVector()) + this->g_log.warning() << "Colinear Assembly.\n"; + + return normal; +} + +// Recursively set all detectors and subcomponents of a bank as visited +void PanelsSurfaceCalculator::setBankVisited(const ComponentInfo &componentInfo, size_t bankIndex, + std::vector &visitedComponents) const { + const auto &children = componentInfo.children(bankIndex); + visitedComponents[bankIndex] = true; + for (auto child : children) { + const auto &subChildren = componentInfo.children(child); + if (subChildren.size() > 0) + setBankVisited(componentInfo, child, visitedComponents); + else + visitedComponents[child] = true; + } +} + +size_t PanelsSurfaceCalculator::findNumDetectors(const ComponentInfo &componentInfo, + const std::vector &components) const { + return std::accumulate( + components.cbegin(), components.cend(), std::size_t{0u}, + [&componentInfo](size_t lhs, const auto &comp) { return componentInfo.isDetector(comp) ? lhs + 1u : lhs; }); +} + +/** + * Calculate the rotation needed around the bank's local x and y axes to place a bank on the projection plane + * Perform the rotation in two stages to avoid any twist about the normal + * + * @param detPos :: Position of a detector of the bank. + * @param normal :: Normal to the bank's plane. + */ +Mantid::Kernel::Quat PanelsSurfaceCalculator::calcBankRotation(const V3D &detPos, V3D normal, const V3D &zAxis, + const V3D &yAxis, const V3D &samplePosition) const { + V3D directionToViewer = zAxis; + V3D bankToOrigin = samplePosition - detPos; + // signed shortest distance from the bank's plane to the origin (m_pos) + double a = normal.scalar_prod(bankToOrigin); + // if a is negative the origin is on the "back" side of the plane + // (the "front" side is facing in the direction of the normal) + if (a < 0.0) { + // we need to flip the normal to make the side looking at the origin to be + // the front one + normal *= -1; + } + double b = zAxis.scalar_prod(bankToOrigin); + if (b < 0.0) { + // if the bank is at positive z then we need to rotate the normal to point in negative z direction + directionToViewer *= -1; + } + if (directionToViewer == -normal) { + return Mantid::Kernel::Quat(0, yAxis.X(), yAxis.Y(), yAxis.Z()); // 180 degree rotation about y axis + } else if (normal.cross_prod(directionToViewer).nullVector()) { + return Mantid::Kernel::Quat(); + } + + Mantid::Kernel::Quat requiredRotation; + if (normal.cross_prod(yAxis).nullVector()) { + requiredRotation = Mantid::Kernel::Quat(normal, directionToViewer); + } else { + V3D normalInXZPlane = {normal.X(), 0., normal.Z()}; + normalInXZPlane.normalize(); + auto rotationLocalX = Mantid::Kernel::Quat(normal, normalInXZPlane); + auto rotAboutY180 = Mantid::Kernel::Quat(0, yAxis.X(), yAxis.Y(), yAxis.Z()); + auto rotationLocalY = + normalInXZPlane == -directionToViewer ? rotAboutY180 : Mantid::Kernel::Quat(normalInXZPlane, directionToViewer); + requiredRotation = rotationLocalY * rotationLocalX; + } + return requiredRotation; +} + +} // namespace Mantid::API diff --git a/Framework/API/test/PanelsSurfaceCalculatorTest.h b/Framework/API/test/PanelsSurfaceCalculatorTest.h new file mode 100644 index 000000000000..6bd25b4233c8 --- /dev/null +++ b/Framework/API/test/PanelsSurfaceCalculatorTest.h @@ -0,0 +1,116 @@ +// Mantid Repository : https://github.com/mantidproject/mantid +// +// Copyright © 2025 ISIS Rutherford Appleton Laboratory UKRI, +// NScD Oak Ridge National Laboratory, European Spallation Source, +// Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS +// SPDX - License - Identifier: GPL - 3.0 + +#pragma once + +#include "MantidFrameworkTestHelpers/WorkspaceCreationHelper.h" +#include +#include +#include +#include + +using Mantid::API::PanelsSurfaceCalculator; + +class PanelsSurfaceCalculatorTest : public CxxTest::TestSuite { +public: + // This pair of boilerplate methods prevent the suite being created statically + // This means the constructor isn't called when running other tests + static PanelsSurfaceCalculatorTest *createSuite() { return new PanelsSurfaceCalculatorTest(); } + static void destroySuite(PanelsSurfaceCalculatorTest *suite) { delete suite; } + + void testCreation() { TS_ASSERT_THROWS_NOTHING(const auto p = PanelsSurfaceCalculator()); } + + void testSetupBasisAxes() { + const auto p = PanelsSurfaceCalculator(); + const auto zAxis = V3D(0, 0, 1); + V3D xAxis, yAxis; + p.setupBasisAxes(zAxis, xAxis, yAxis); + // Need to check that the generated x and y axes form a basis + const double tol = 1e-9; + TS_ASSERT_DELTA(0, zAxis.scalar_prod(xAxis), tol); + TS_ASSERT_DELTA(0, zAxis.scalar_prod(yAxis), tol); + TS_ASSERT_DELTA(0, xAxis.scalar_prod(yAxis), tol); + TS_ASSERT_DELTA(1, xAxis.scalar_prod(xAxis), tol); + TS_ASSERT_DELTA(1, zAxis.scalar_prod(zAxis), tol); + TS_ASSERT_DELTA(1, yAxis.scalar_prod(yAxis), tol); + } + + void testRetrievePanelCorners() { + const auto ws = WorkspaceCreationHelper::create2DWorkspaceWithRectangularInstrument(1, 5, 10); + const size_t rectangularComponentIndex = 30; + const auto p = PanelsSurfaceCalculator(); + const auto corners = p.retrievePanelCorners(ws->componentInfo(), rectangularComponentIndex); + const double tol = 1e-4; + compareTwoV3Ds(V3D(0, -0.5, 5), corners[0], tol); + compareTwoV3Ds(V3D(0.032, -0.5, 5), corners[1], tol); + compareTwoV3Ds(V3D(0.032, 0.53205, 5), corners[2], tol); + compareTwoV3Ds(V3D(0, 0.53205, 5), corners[3], tol); + } + + void testCalculatePanelNormal() { + const auto ws = WorkspaceCreationHelper::create2DWorkspaceWithRectangularInstrument(1, 5, 10); + const size_t rectangularComponentIndex = 30; + const auto p = PanelsSurfaceCalculator(); + const auto corners = p.retrievePanelCorners(ws->componentInfo(), rectangularComponentIndex); + const auto normal = p.calculatePanelNormal(corners); + compareTwoV3Ds(V3D(0, 0, 1), normal, 1e-4); + } + + void testIsBankFlat() { + const auto ws = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(1, 10); + const size_t tubeIndex = 4; + auto p = PanelsSurfaceCalculator(); + const V3D normal(0, 1, 0); + const std::vector tubes{tubeIndex}; + const bool isBankFlat = p.isBankFlat(ws->componentInfo(), tubeIndex, tubes, normal); + TS_ASSERT(isBankFlat); + } + + void testCalculateBankNormal() { + const auto ws = WorkspaceCreationHelper::create2DWorkspaceWithRectangularInstrument(1, 5, 10); + auto p = PanelsSurfaceCalculator(); + const auto bankNormal = p.calculateBankNormal(ws->componentInfo(), {25, 26}); + compareTwoV3Ds(V3D(0, 0, -1), bankNormal, 1e-9); + } + + void testSetBankVisited() { + const auto ws = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(1, 10); + const size_t tubeIndex = 4; + auto p = PanelsSurfaceCalculator(); + std::vector visitedComponents(ws->componentInfo().size(), false); + p.setBankVisited(ws->componentInfo(), tubeIndex, visitedComponents); + TS_ASSERT(visitedComponents[tubeIndex]); + } + + void testFindNumDetectors() { + const auto ws = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(1, 10); + auto p = PanelsSurfaceCalculator(); + std::vector components(ws->componentInfo().size()); + std::iota(components.begin(), components.end(), 0); + const auto numDetectors = p.findNumDetectors(ws->componentInfo(), components); + TS_ASSERT_EQUALS(1, numDetectors); + } + + void testCalcBankRotation() { + const V3D detectorPosition(1, 0, 0), normal(0, 1, 0), zAxis(0, 0, 1), yAxis(0, 1, 0), samplePosition(0, 0, 0); + auto p = PanelsSurfaceCalculator(); + const auto rotation = p.calcBankRotation(detectorPosition, normal, zAxis, yAxis, samplePosition); + const double pi = std::acos(-1); + const double angle = std::cos(pi / 4.0); + const double tol = 1e-9; + TS_ASSERT_DELTA(angle, rotation.real(), tol); + TS_ASSERT_DELTA(angle, rotation.imagI(), tol); + TS_ASSERT_DELTA(0, rotation.imagJ(), tol); + TS_ASSERT_DELTA(0, rotation.imagK(), tol); + } + +private: + void compareTwoV3Ds(const V3D &a, const V3D &b, const double tol) { + for (size_t i = 0; i < 3; i++) { + TS_ASSERT_DELTA(a[i], b[i], tol); + } + } +}; diff --git a/Framework/PythonInterface/mantid/api/CMakeLists.txt b/Framework/PythonInterface/mantid/api/CMakeLists.txt index 6a3d8e059197..9ef304d123bf 100644 --- a/Framework/PythonInterface/mantid/api/CMakeLists.txt +++ b/Framework/PythonInterface/mantid/api/CMakeLists.txt @@ -90,6 +90,7 @@ set(EXPORT_FILES src/Exports/PreviewManager.cpp src/Exports/RegionSelectorObserver.cpp src/Exports/WorkspaceNearestNeighbourInfo.cpp + src/Exports/PanelsSurfaceCalculator.cpp ) set(MODULE_DEFINITION ${CMAKE_CURRENT_BINARY_DIR}/api.cpp) diff --git a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp new file mode 100644 index 000000000000..1c5897913935 --- /dev/null +++ b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp @@ -0,0 +1,17 @@ +// Mantid Repository : https://github.com/mantidproject/mantid +// +// Copyright © 2025 ISIS Rutherford Appleton Laboratory UKRI, +// NScD Oak Ridge National Laboratory, European Spallation Source, +// Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS +// SPDX - License - Identifier: GPL - 3.0 + +#pragma once + +#include "MantidAPI/PanelsSurfaceCalculator.h" +#include + +void export_PanelsSurfaceCalculator() { + using namespace boost::python; + using Mantid::API::PanelsSurfaceCalculator; + + class_("PanelsSurfaceCalculator", no_init); +} diff --git a/qt/widgets/instrumentview/inc/MantidQtWidgets/InstrumentView/PanelsSurface.h b/qt/widgets/instrumentview/inc/MantidQtWidgets/InstrumentView/PanelsSurface.h index 6da0cac8e268..4b962730b71d 100644 --- a/qt/widgets/instrumentview/inc/MantidQtWidgets/InstrumentView/PanelsSurface.h +++ b/qt/widgets/instrumentview/inc/MantidQtWidgets/InstrumentView/PanelsSurface.h @@ -6,6 +6,7 @@ // SPDX - License - Identifier: GPL - 3.0 + #pragma once +#include "MantidAPI/PanelsSurfaceCalculator.h" #include "UnwrappedSurface.h" #include @@ -81,7 +82,6 @@ class EXPORT_OPT_MANTIDQT_INSTRUMENTVIEW PanelsSurface : public UnwrappedSurface void setupAxes(); // Add a flat bank void constructFromComponentInfo(); - Mantid::Kernel::Quat calcBankRotation(const Mantid::Kernel::V3D &detPos, Mantid::Kernel::V3D normal) const; // Add a detector from an assembly void addDetector(size_t detIndex, int bankIndex); // Arrange the banks on the projection plane @@ -112,6 +112,9 @@ class EXPORT_OPT_MANTIDQT_INSTRUMENTVIEW PanelsSurface : public UnwrappedSurface std::vector m_detector2bankMap; friend struct FlatBankInfo; + +private: + Mantid::API::PanelsSurfaceCalculator m_calculator; }; } // namespace MantidWidgets diff --git a/qt/widgets/instrumentview/src/PanelsSurface.cpp b/qt/widgets/instrumentview/src/PanelsSurface.cpp index d2c854418fca..bf5e7c10713c 100644 --- a/qt/widgets/instrumentview/src/PanelsSurface.cpp +++ b/qt/widgets/instrumentview/src/PanelsSurface.cpp @@ -35,150 +35,6 @@ namespace { /// static logger Mantid::Kernel::Logger g_log("PanelsSurface"); -/** - * Given the z axis, define the x and y ones. - * @param zaxis :: A given vector in 3d space to become the z axis of a - * coordinate system. - * @param xaxis :: An output arbitrary vector perpendicular to zaxis. - * @param yaxis :: An output arbitrary vector perpendicular to both zaxis and - * xaxis. - */ -void setupBasisAxes(const Mantid::Kernel::V3D &zaxis, Mantid::Kernel::V3D &xaxis, Mantid::Kernel::V3D &yaxis) { - double R, theta, phi; - zaxis.getSpherical(R, theta, phi); - if (theta <= 45.0) { - xaxis = Mantid::Kernel::V3D(1, 0, 0); - } else if (phi <= 45.0) { - xaxis = Mantid::Kernel::V3D(0, 1, 0); - } else { - xaxis = Mantid::Kernel::V3D(0, 0, 1); - } - yaxis = zaxis.cross_prod(xaxis); - yaxis.normalize(); - xaxis = yaxis.cross_prod(zaxis); -} - -std::vector retrievePanelCorners(const Mantid::Geometry::ComponentInfo &componentInfo, - size_t rootIndex) { - auto panel = componentInfo.quadrilateralComponent(rootIndex); - auto child = rootIndex; - - // Find shapeInfo for one of the children - do { - const auto &children = componentInfo.children(child); - child = children[0]; - } while (!componentInfo.isDetector(child)); - - const auto &shape = componentInfo.shape(child); - const auto &shapeInfo = shape.getGeometryHandler()->shapeInfo(); - const auto &points = shapeInfo.points(); - double xoff = 0.0; - double yoff = 0.0; - - // Find x and y widths of detectors and treat as offsets; - for (size_t i = 0; i < points.size() - 1; ++i) { - double xdiff = std::abs(points[i + 1].X() - points[i].X()); - double ydiff = std::abs(points[i + 1].Y() - points[i].Y()); - if (xdiff != 0) - xoff = xdiff * 0.5; - if (ydiff != 0) - yoff = ydiff * 0.5; - } - - std::vector corners{ - componentInfo.position(panel.bottomLeft), componentInfo.position(panel.bottomRight), - componentInfo.position(panel.topRight), componentInfo.position(panel.topLeft)}; - - // Find xmin, xmax, ymin and ymax - double xmin = corners[0].X(); - double xmax = corners[0].X(); - double ymin = corners[0].Y(); - double ymax = corners[0].Y(); - - for (const auto &corner : corners) { - xmin = corner.X() < xmin ? corner.X() : xmin; - xmax = corner.X() > xmax ? corner.X() : xmax; - ymin = corner.Y() < ymin ? corner.Y() : ymin; - ymax = corner.Y() > ymax ? corner.Y() : ymax; - } - - // apply offsets - for (auto &corner : corners) { - auto x = corner.X(); - auto y = corner.Y(); - corner.setX(x == xmin ? x - xoff : x + xoff); - corner.setY(y == ymin ? y - yoff : y + yoff); - } - - return corners; -} - -Mantid::Kernel::V3D calculatePanelNormal(const std::vector &panelCorners) { - // find the normal - auto xaxis = panelCorners[1] - panelCorners[0]; - auto yaxis = panelCorners[3] - panelCorners[0]; - const auto normal = normalize(xaxis.cross_prod(yaxis)); - return normal; -} - -bool isBankFlat(const ComponentInfo &componentInfo, size_t bankIndex, const std::vector &tubes, - const Mantid::Kernel::V3D &normal) { - for (auto tube : tubes) { - const auto &children = componentInfo.children(tube); - const auto vector = normalize(componentInfo.position(children[0]) - componentInfo.position(children[1])); - if (fabs(vector.scalar_prod(normal)) > Mantid::Kernel::Tolerance) { - g_log.warning() << "Assembly " << componentInfo.name(bankIndex) << " isn't flat.\n"; - return false; - } - } - return true; -} - -Mantid::Kernel::V3D calculateBankNormal(const ComponentInfo &componentInfo, const std::vector &tubes) { - // calculate normal from first two tubes in bank as before - const auto &tube0 = componentInfo.children(tubes[0]); - const auto &tube1 = componentInfo.children(tubes[1]); - auto pos = componentInfo.position(tube0[0]); - auto x = componentInfo.position(tube0[1]) - pos; - x.normalize(); - - auto y = componentInfo.position(tube1[0]) - pos; - y.normalize(); - auto normal = x.cross_prod(y); - - if (normal.nullVector()) { - y = componentInfo.position(tube1[1]) - componentInfo.position(tube1[0]); - y.normalize(); - normal = x.cross_prod(y); - } - - normal.normalize(); - - if (normal.nullVector()) - g_log.warning() << "Colinear Assembly.\n"; - - return normal; -} - -// Recursively set all detectors and subcomponents of a bank as visited -void setBankVisited(const ComponentInfo &componentInfo, size_t bankIndex, std::vector &visitedComponents) { - const auto &children = componentInfo.children(bankIndex); - visitedComponents[bankIndex] = true; - for (auto child : children) { - const auto &subChildren = componentInfo.children(child); - if (subChildren.size() > 0) - setBankVisited(componentInfo, child, visitedComponents); - else - visitedComponents[child] = true; - } -} - -size_t findNumDetectors(const ComponentInfo &componentInfo, const std::vector &components) { - return std::accumulate( - components.cbegin(), components.cend(), std::size_t{0u}, - [&componentInfo](size_t lhs, const auto &comp) { return componentInfo.isDetector(comp) ? lhs + 1u : lhs; }); -} - void initialisePolygonWithTransformedBoundingBoxPoints(QPolygonF &panelPolygon, const ComponentInfo &componentInfo, size_t detectorIndex, const V3D &refPos, const Quat &rotation, const V3D &xaxis, const V3D &yaxis) { @@ -332,7 +188,7 @@ void PanelsSurface::rotate(const UnwrappedDetector &udet, Mantid::Kernel::Quat & * Define a coordinate system for this projection. */ void PanelsSurface::setupAxes() { - setupBasisAxes(m_zaxis, m_xaxis, m_yaxis); + this->m_calculator.setupBasisAxes(m_zaxis, m_xaxis, m_yaxis); m_origin.rx() = m_xaxis.scalar_prod(m_pos); m_origin.ry() = m_yaxis.scalar_prod(m_pos); } @@ -342,15 +198,15 @@ void PanelsSurface::setupAxes() { void PanelsSurface::processStructured(size_t rootIndex) { int index = m_flatBanks.size(); const auto &componentInfo = m_instrActor->componentInfo(); - auto corners = retrievePanelCorners(componentInfo, rootIndex); - auto normal = calculatePanelNormal(corners); + auto corners = this->m_calculator.retrievePanelCorners(componentInfo, rootIndex); + auto normal = this->m_calculator.calculatePanelNormal(corners); // save bank info auto *info = new FlatBankInfo(this); m_flatBanks << info; auto ref = corners[0]; info->refPos = ref; // find the rotation to put the bank on the plane - info->rotation = calcBankRotation(ref, normal); + info->rotation = this->m_calculator.calcBankRotation(ref, normal, m_zaxis, m_yaxis, m_pos); const auto &columns = componentInfo.children(rootIndex); const auto &firstRow = componentInfo.children(columns.front()); const auto &lastRow = componentInfo.children(columns.back()); @@ -427,10 +283,10 @@ std::optional PanelsSurface::processTubes(size_t rootIndex) { return bankIndex; // Now we found all the tubes that may form a flat struture. // Use two of the tubes to calculate the normal to the plain of that structure - normal = tubes.size() > 1 ? calculateBankNormal(componentInfo, tubes) : V3D(); + normal = tubes.size() > 1 ? this->m_calculator.calculateBankNormal(componentInfo, tubes) : V3D(); // If some of the tubes are not perpendicular to the normal the structure // isn't flat - if (!normal.nullVector() && isBankFlat(componentInfo, bankIndex, tubes, normal)) + if (!normal.nullVector() && this->m_calculator.isBankFlat(componentInfo, bankIndex, tubes, normal)) foundFlatBank = true; } @@ -439,8 +295,8 @@ std::optional PanelsSurface::processTubes(size_t rootIndex) { tubes.clear(); bankIndex = bankIndex0; addTubes(bankIndex, tubes); - normal = tubes.size() > 1 ? calculateBankNormal(componentInfo, tubes) : V3D(); - if (normal.nullVector() || !isBankFlat(componentInfo, bankIndex, tubes, normal)) + normal = tubes.size() > 1 ? this->m_calculator.calculateBankNormal(componentInfo, tubes) : V3D(); + if (normal.nullVector() || !this->m_calculator.isBankFlat(componentInfo, bankIndex, tubes, normal)) return std::nullopt; } @@ -461,7 +317,7 @@ std::optional PanelsSurface::processTubes(size_t rootIndex) { auto pos1 = componentInfo.position(componentInfo.children(tubes.front()).back()); info->refPos = pos0; - info->rotation = calcBankRotation(pos0, normal); + info->rotation = this->m_calculator.calcBankRotation(pos0, normal, m_zaxis, m_yaxis, m_pos); pos1 -= pos0; info->rotation.rotate(pos1); pos1 += pos0; @@ -513,7 +369,7 @@ void PanelsSurface::processUnstructured(size_t rootIndex, std::vector &vis bool normalFound = false; const auto &componentInfo = m_instrActor->componentInfo(); const auto &children = componentInfo.children(rootIndex); - auto numDets = findNumDetectors(componentInfo, children); + auto numDets = this->m_calculator.findNumDetectors(componentInfo, children); if (numDets == 0) return; @@ -532,7 +388,7 @@ void PanelsSurface::processUnstructured(size_t rootIndex, std::vector &vis // at first set the normal to an arbitrary vector orthogonal to // the line between the first two detectors y = normalize(pos - pos0); - setupBasisAxes(y, normal, x); + this->m_calculator.setupBasisAxes(y, normal, x); } else if (fabs(normal.scalar_prod(pos - pos0)) > Mantid::Kernel::Tolerance) { if (!normalFound) { // when first non-colinear detector is found set the normal @@ -560,7 +416,7 @@ void PanelsSurface::processUnstructured(size_t rootIndex, std::vector &vis // keep reference position on the bank's plane auto pos1 = detectorInfo.position(detectors[1]) - pos0; - info->rotation = calcBankRotation(pos0, normal); + info->rotation = this->m_calculator.calcBankRotation(pos0, normal, m_zaxis, m_yaxis, m_pos); info->rotation.rotate(pos1); pos1 += pos0; QPointF p0(m_xaxis.scalar_prod(pos0), m_yaxis.scalar_prod(pos0)); @@ -600,23 +456,23 @@ void PanelsSurface::findFlatPanels(size_t rootIndex, std::vector &visited) componentType = componentInfo.componentType(rootIndex); if (componentType == ComponentType::Rectangular || componentType == ComponentType::Structured) { processStructured(rootIndex); - setBankVisited(componentInfo, rootIndex, visited); + this->m_calculator.setBankVisited(componentInfo, rootIndex, visited); return; } if (componentType == ComponentType::OutlineComposite) { const auto bankIndex = processTubes(rootIndex); if (bankIndex) { - setBankVisited(componentInfo, bankIndex.value(), visited); + this->m_calculator.setBankVisited(componentInfo, bankIndex.value(), visited); } else { - setBankVisited(componentInfo, parentIndex, visited); + this->m_calculator.setBankVisited(componentInfo, parentIndex, visited); } return; } if (componentType == ComponentType::Grid) { processGrid(rootIndex); - setBankVisited(componentInfo, rootIndex, visited); + this->m_calculator.setBankVisited(componentInfo, rootIndex, visited); return; } @@ -639,53 +495,6 @@ void PanelsSurface::constructFromComponentInfo() { } } -/** - * Calculate the rotation needed around the bank's local x and y axes to place a bank on the projection plane - * Perform the rotation in two stages to avoid any twist about the normal - * - * @param detPos :: Position of a detector of the bank. - * @param normal :: Normal to the bank's plane. - */ -Mantid::Kernel::Quat PanelsSurface::calcBankRotation(const Mantid::Kernel::V3D &detPos, - Mantid::Kernel::V3D normal) const { - - V3D directionToViewer = m_zaxis; - V3D bankToOrigin = m_pos - detPos; - // signed shortest distance from the bank's plane to the origin (m_pos) - double a = normal.scalar_prod(bankToOrigin); - // if a is negative the origin is on the "back" side of the plane - // (the "front" side is facing in the direction of the normal) - if (a < 0.0) { - // we need to flip the normal to make the side looking at the origin to be - // the front one - normal *= -1; - } - double b = m_zaxis.scalar_prod(bankToOrigin); - if (b < 0.0) { - // if the bank is at positive z then we need to rotate the normal to point in negative z direction - directionToViewer *= -1; - } - if (directionToViewer == -normal) { - return Mantid::Kernel::Quat(0, m_yaxis.X(), m_yaxis.Y(), m_yaxis.Z()); // 180 degree rotation about y axis - } else if (normal.cross_prod(directionToViewer).nullVector()) { - return Mantid::Kernel::Quat(); - } - - Quat requiredRotation; - if (normal.cross_prod(m_yaxis).nullVector()) { - requiredRotation = Mantid::Kernel::Quat(normal, directionToViewer); - } else { - Mantid::Kernel::V3D normalInXZPlane = {normal.X(), 0., normal.Z()}; - normalInXZPlane.normalize(); - auto rotationLocalX = Mantid::Kernel::Quat(normal, normalInXZPlane); - auto rotAboutY180 = Mantid::Kernel::Quat(0, m_yaxis.X(), m_yaxis.Y(), m_yaxis.Z()); - auto rotationLocalY = - normalInXZPlane == -directionToViewer ? rotAboutY180 : Mantid::Kernel::Quat(normalInXZPlane, directionToViewer); - requiredRotation = rotationLocalY * rotationLocalX; - } - return requiredRotation; -} - void PanelsSurface::addDetector(size_t detIndex, int bankIndex) { m_detector2bankMap[detIndex] = bankIndex; // get the colour diff --git a/qt/widgets/instrumentview/test/InstrumentWidget/PanelsSurfaceTest.h b/qt/widgets/instrumentview/test/InstrumentWidget/PanelsSurfaceTest.h index 47821fee9c94..f348f0d74dd8 100644 --- a/qt/widgets/instrumentview/test/InstrumentWidget/PanelsSurfaceTest.h +++ b/qt/widgets/instrumentview/test/InstrumentWidget/PanelsSurfaceTest.h @@ -32,7 +32,7 @@ class PanelsSurfaceHelper : public PanelsSurface { PanelsSurface::project(position, u, v, uscale, vscale); } Mantid::Kernel::Quat calcBankRotation(const Mantid::Kernel::V3D &detPos, Mantid::Kernel::V3D normal) { - return PanelsSurface::calcBankRotation(detPos, normal); + return this->m_calculator.calcBankRotation(detPos, normal, m_zaxis, m_yaxis, m_pos); } std::optional processTubes(size_t rootIndex) { return PanelsSurface::processTubes(rootIndex); } std::vector getUnwrappedDetectors() { return m_unwrappedDetectors; }; @@ -42,6 +42,9 @@ class PanelsSurfaceHelper : public PanelsSurface { info->refPos = refPos; m_flatBanks << info; } + +private: + PanelsSurfaceCalculator m_calculator; }; class PanelsSurfaceTest : public CxxTest::TestSuite { From 916a805a9c33c589f8d1cf5ef1a9f47c738ac3f6 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Thu, 10 Jul 2025 14:50:43 +0100 Subject: [PATCH 02/23] Add Python interface to new calculator class Also added tests for the Python interface. --- .../src/Exports/PanelsSurfaceCalculator.cpp | 138 +++++++++++++++++- .../test/python/mantid/api/CMakeLists.txt | 1 + .../mantid/api/PanelsSurfaceCalculatorTest.py | 78 ++++++++++ 3 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py diff --git a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp index 1c5897913935..7e7c5ff2bc31 100644 --- a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp +++ b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp @@ -7,11 +7,147 @@ #pragma once #include "MantidAPI/PanelsSurfaceCalculator.h" +#include "MantidAPI/MatrixWorkspace.h" +#include #include +#include + +using namespace boost::python; + +namespace { +using Mantid::API::PanelsSurfaceCalculator; + +V3D pythonListToV3D(const object &pyList) { + size_t listLength = len(pyList); + if (listLength != 3) { + throw std::invalid_argument("List must have length 3."); + } + return {extract(pyList[0]), extract(pyList[1]), extract(pyList[2])}; +} + +void setupBasisAxes(PanelsSurfaceCalculator &self, list &xAxis, list &yAxis, const list &zAxis) { + auto x = pythonListToV3D(xAxis); + auto y = pythonListToV3D(yAxis); + const auto z = pythonListToV3D(zAxis); + self.setupBasisAxes(z, x, y); + for (size_t i = 0; i < 3; i++) { + xAxis[i] = x[i]; + yAxis[i] = y[i]; + } +} + +list retrievePanelCorners(PanelsSurfaceCalculator &self, const object &componentInfo, const size_t rootIndex) { + const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); + const auto panelCorners = self.retrievePanelCorners(*(cInfoSharedPtr.get()), rootIndex); + list panelCornersList; + for (size_t i = 0; i < panelCorners.size(); i++) { + list corner(panelCorners[i]); + panelCornersList.append(corner); + } + return panelCornersList; +} + +list calculatePanelNormal(PanelsSurfaceCalculator &self, const list &panelCorners) { + if (len(panelCorners) != 4) { + throw std::invalid_argument("Must be 4 panel corners"); + } + std::vector panelCornersVec{4}; + for (size_t i = 0; i < 4; i++) { + const object corner = panelCorners[i]; + panelCornersVec[i] = pythonListToV3D(panelCorners[i]); + } + const auto panelNormal = self.calculatePanelNormal(panelCornersVec); + list panelNormalList(panelNormal); + return panelNormalList; +} + +bool isBankFlat(PanelsSurfaceCalculator &self, const object &componentInfo, const size_t bankIndex, const list &tubes, + const list &normal) { + const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); + std::vector tubesVector; + for (size_t i = 0; i < len(tubes); i++) { + tubesVector.push_back(extract(tubes[i])); + } + const auto normalV3D = pythonListToV3D(normal); + return self.isBankFlat(*(cInfoSharedPtr.get()), bankIndex, tubesVector, normalV3D); +} + +list calculateBankNormal(PanelsSurfaceCalculator &self, const object &componentInfo, const list &tubes) { + const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); + std::vector tubesVector; + for (size_t i = 0; i < len(tubes); i++) { + tubesVector.push_back(extract(tubes[i])); + } + const auto normal = self.calculateBankNormal(*(cInfoSharedPtr.get()), tubesVector); + list normalList{normal}; + return normalList; +} + +void setBankVisited(PanelsSurfaceCalculator &self, const object &componentInfo, const size_t bankIndex, + list &visitedComponents) { + const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); + std::vector visitedComponentsVector; + for (size_t i = 0; i < len(visitedComponents); i++) { + visitedComponentsVector.push_back(extract(visitedComponents[i])); + } + self.setBankVisited(*(cInfoSharedPtr.get()), bankIndex, visitedComponentsVector); + for (size_t i = 0; i < len(visitedComponents); i++) { + // A bool is 8 bytes because a byte is the smallest addressable unit of memory, but + // an std::vector uses bits to store bools as a memory optimisation. Hence + // the objects in a vector are not actually bools, and boost::python + // doesn't know what to do with them + visitedComponents[i] = visitedComponentsVector[i] ? true : false; + } +} + +size_t findNumDetectors(PanelsSurfaceCalculator &self, const object &componentInfo, const list &components) { + const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); + std::vector componentsVector; + for (size_t i = 0; i < len(components); i++) { + componentsVector.push_back(extract(components[i])); + } + return self.findNumDetectors(*(cInfoSharedPtr.get()), componentsVector); +} + +list calcBankRotation(PanelsSurfaceCalculator &self, const list &detPos, list normal, const list &zAxis, + const list &yAxis, const list &samplePosition) { + const auto bankRotation = + self.calcBankRotation(pythonListToV3D(detPos), pythonListToV3D(normal), pythonListToV3D(zAxis), + pythonListToV3D(yAxis), pythonListToV3D(samplePosition)); + + list quat; + quat.append(bankRotation.real()); + quat.append(bankRotation.imagI()); + quat.append(bankRotation.imagJ()); + quat.append(bankRotation.imagK()); + return quat; +} + +} // namespace void export_PanelsSurfaceCalculator() { using namespace boost::python; using Mantid::API::PanelsSurfaceCalculator; - class_("PanelsSurfaceCalculator", no_init); + class_("PanelsSurfaceCalculator", + init<>("Make a side by side projection calculator")) + .def("setupBasisAxes", &setupBasisAxes, (arg("self"), arg("xaxis"), arg("yaxis"), arg("zaxis")), + "Sets up the basis axes for the projection.") + .def("retrievePanelCorners", &retrievePanelCorners, (arg("self"), arg("componentInfo"), arg("rootIndex")), + "Retrieves the corners of the panel.") + .def("calculatePanelNormal", &calculatePanelNormal, (arg("self"), arg("panelCorners")), + "Calculates the normal vector of the panel.") + .def("isBankFlat", &isBankFlat, + (arg("self"), arg("componentInfo"), arg("bankIndex"), arg("tubes"), arg("normal")), + "Checks if a bank is flat based on its normal vector.") + .def("calculateBankNormal", &calculateBankNormal, (arg("self"), arg("componentInfo"), arg("tubes")), + "Calculates the normal vector of a bank.") + .def("setBankVisited", &setBankVisited, + (arg("self"), arg("componentInfo"), arg("bankIndex"), arg("visitedComponents")), + "Marks a bank as visited in the visitedComponents vector") + .def("findNumDetectors", &findNumDetectors, (arg("componentInfo"), arg("components")), + "Finds the number of detectors in a component.") + .def("calcBankRotation", &calcBankRotation, + (arg("self"), arg("detPos"), arg("normal"), arg("zAxis"), arg("yAxis"), arg("samplePosition")), + "Calculates the rotation quaternion for a bank based on its position and normal vector."); } diff --git a/Framework/PythonInterface/test/python/mantid/api/CMakeLists.txt b/Framework/PythonInterface/test/python/mantid/api/CMakeLists.txt index 304f82c3034a..e1e8fd7dec9c 100644 --- a/Framework/PythonInterface/test/python/mantid/api/CMakeLists.txt +++ b/Framework/PythonInterface/test/python/mantid/api/CMakeLists.txt @@ -36,6 +36,7 @@ set(TEST_PY_FILES MDHistoWorkspaceTest.py MultipleExperimentInfos.py MultipleFilePropertyTest.py + PanelsSurfaceCalculatorTest.py PreviewManagerTest.py ProgressTest.py ProjectionTest.py diff --git a/Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py b/Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py new file mode 100644 index 000000000000..7eb2b7ad1425 --- /dev/null +++ b/Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py @@ -0,0 +1,78 @@ +# Mantid Repository : https://github.com/mantidproject/mantid +# +# Copyright © 2025 ISIS Rutherford Appleton Laboratory UKRI, +# NScD Oak Ridge National Laboratory, European Spallation Source, +# Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS +# SPDX - License - Identifier: GPL - 3.0 + +import unittest + +from mantid.api import PanelsSurfaceCalculator +from mantid.simpleapi import CreateSampleWorkspace +import numpy as np + + +class PanelsSurfaceCalculatorTest(unittest.TestCase): + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + cls._ws = CreateSampleWorkspace(NumBanks=1, BankPixelWidth=5, StoreInADS=False) + cls._calculator = PanelsSurfaceCalculator() + + def test_calculator_initialization(self): + self.assertIsNotNone(self._calculator) + + def test_setupBasisAxes(self): + z_axis = [0, 0, 1] + x_axis, y_axis = [0, 0, 0], [0, 0, 0] + self._calculator.setupBasisAxes(x_axis, y_axis, z_axis) + self.assertEqual(x_axis, [1, 0, 0]) + self.assertEqual(y_axis, [0, 1, 0]) + self.assertEqual(z_axis, [0, 0, 1]) + + def test_retrievePanelCorners(self): + corners = self._calculator.retrievePanelCorners(self._ws.componentInfo(), 30) + self.assertEqual(4, len(corners)) + absolute_tolerance = 1e-4 + np.testing.assert_allclose([0, -0.5, 5], corners[0], atol=absolute_tolerance) + np.testing.assert_allclose([0.032, -0.5, 5], corners[1], atol=absolute_tolerance) + np.testing.assert_allclose([0.032, 0.53205, 5], corners[2], atol=absolute_tolerance) + np.testing.assert_allclose([0, 0.53205, 5], corners[3], atol=absolute_tolerance) + + def test_calculatePanelNormal(self): + corners = self._calculator.retrievePanelCorners(self._ws.componentInfo(), 30) + normal = self._calculator.calculatePanelNormal(corners) + np.testing.assert_allclose([0, 0, 1], normal, 1e-4) + + def test_isBankFlat(self): + normal = [0, 1, 0] + is_bank_flat = self._calculator.isBankFlat(self._ws.componentInfo(), 25, [25], normal) + self.assertFalse(is_bank_flat) + + def test_calculateBankNormal(self): + normal = self._calculator.calculateBankNormal(self._ws.componentInfo(), [25, 26]) + np.testing.assert_allclose([0, 0, -1], normal, atol=1e-4) + + def test_setBankVisited(self): + visited_components = [False for i in range(self._ws.componentInfo().size())] + self._calculator.setBankVisited(self._ws.componentInfo(), 4, visited_components) + self.assertTrue(visited_components[4]) + + def test_findNumDetectors(self): + components = list(range(self._ws.componentInfo().size())) + num_detectors = self._calculator.findNumDetectors(self._ws.componentInfo(), components) + self.assertEqual(25, num_detectors) + + def test_calcBankRotation(self): + detector_Position = [1, 0, 0] + normal = [0, 1, 0] + z_axis = [0, 0, 1] + y_axis = [0, 1, 0] + sample_position = [0, 0, 0] + rotation = self._calculator.calcBankRotation(detector_Position, normal, z_axis, y_axis, sample_position) + angle = np.cos(np.pi / 4) + expected_rotation = [angle, angle, 0, 0] + np.testing.assert_allclose(expected_rotation, rotation, atol=1e-9) + + +if __name__ == "__main__": + unittest.main() From da97f78bc7a83fa8c4a9ac376ee8877ef86d323a Mon Sep 17 00:00:00 2001 From: James Clarke Date: Thu, 10 Jul 2025 16:45:12 +0100 Subject: [PATCH 03/23] Move more logic to PanelsSurfaceCalculator Added the method to the Python interface, and added C++ and Python tests. --- .../inc/MantidAPI/PanelsSurfaceCalculator.h | 5 ++++ Framework/API/src/PanelsSurfaceCalculator.cpp | 16 +++++++++++ .../API/test/PanelsSurfaceCalculatorTest.h | 13 +++++++++ .../src/Exports/PanelsSurfaceCalculator.cpp | 28 ++++++++++++++++++- .../mantid/api/PanelsSurfaceCalculatorTest.py | 11 ++++++++ .../instrumentview/src/PanelsSurface.cpp | 25 ++++------------- 6 files changed, 78 insertions(+), 20 deletions(-) diff --git a/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h index e93d413b8619..ee15d33a177d 100644 --- a/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h +++ b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h @@ -10,6 +10,7 @@ #include "MantidGeometry/Instrument/ComponentInfo.h" #include "MantidKernel/Logger.h" #include "MantidKernel/Quat.h" +#include "MantidKernel/V2D.h" #include "MantidKernel/V3D.h" using Mantid::Geometry::ComponentInfo; @@ -29,6 +30,10 @@ class MANTID_API_DLL PanelsSurfaceCalculator { size_t findNumDetectors(const ComponentInfo &componentInfo, const std::vector &components) const; Mantid::Kernel::Quat calcBankRotation(const V3D &detPos, V3D normal, const V3D &zAxis, const V3D &yAxis, const V3D &samplePosition) const; + std::vector transformedBoundingBoxPoints(const ComponentInfo &componentInfo, + size_t detectorIndex, const V3D &refPos, + const Mantid::Kernel::Quat &rotation, const V3D &xaxis, + const V3D &yaxis) const; private: Mantid::Kernel::Logger g_log = Mantid::Kernel::Logger("PanelsSurfaceCalculator"); diff --git a/Framework/API/src/PanelsSurfaceCalculator.cpp b/Framework/API/src/PanelsSurfaceCalculator.cpp index 197c4d43cae4..ce00fd63040b 100644 --- a/Framework/API/src/PanelsSurfaceCalculator.cpp +++ b/Framework/API/src/PanelsSurfaceCalculator.cpp @@ -213,4 +213,20 @@ Mantid::Kernel::Quat PanelsSurfaceCalculator::calcBankRotation(const V3D &detPos return requiredRotation; } +std::vector +PanelsSurfaceCalculator::transformedBoundingBoxPoints(const ComponentInfo &componentInfo, size_t detectorIndex, + const V3D &refPos, const Mantid::Kernel::Quat &rotation, + const V3D &xaxis, const V3D &yaxis) const { + auto bb = componentInfo.boundingBox(detectorIndex); + auto bbMinPoint = bb.minPoint() - refPos; + auto bbMaxPoint = bb.maxPoint() - refPos; + rotation.rotate(bbMinPoint); + rotation.rotate(bbMaxPoint); + bbMinPoint += refPos; + bbMaxPoint += refPos; + Mantid::Kernel::V2D bb0(xaxis.scalar_prod(bbMinPoint), yaxis.scalar_prod(bbMinPoint)); + Mantid::Kernel::V2D bb1(xaxis.scalar_prod(bbMaxPoint), yaxis.scalar_prod(bbMaxPoint)); + return {bb0, bb1}; +} + } // namespace Mantid::API diff --git a/Framework/API/test/PanelsSurfaceCalculatorTest.h b/Framework/API/test/PanelsSurfaceCalculatorTest.h index 6bd25b4233c8..d1b3b9a577c2 100644 --- a/Framework/API/test/PanelsSurfaceCalculatorTest.h +++ b/Framework/API/test/PanelsSurfaceCalculatorTest.h @@ -107,6 +107,19 @@ class PanelsSurfaceCalculatorTest : public CxxTest::TestSuite { TS_ASSERT_DELTA(0, rotation.imagK(), tol); } + void testTransformedBoundingBoxPoints() { + const auto ws = WorkspaceCreationHelper::create2DWorkspaceWithRectangularInstrument(1, 5, 10); + const auto p = PanelsSurfaceCalculator(); + Mantid::Kernel::Quat rotation(45, {1, 0, 0}); + const auto boundingBoxPoints = + p.transformedBoundingBoxPoints(ws->componentInfo(), 9, {0, 0, 0}, rotation, {1, 0, 0}, {0, 1, 0}); + const double tol = 1e-3; + TS_ASSERT_DELTA(0.004, boundingBoxPoints[0].X(), tol); + TS_ASSERT_DELTA(-3.510, boundingBoxPoints[0].Y(), tol); + TS_ASSERT_DELTA(0.012, boundingBoxPoints[1].X(), tol); + TS_ASSERT_DELTA(-3.516, boundingBoxPoints[1].Y(), tol); + } + private: void compareTwoV3Ds(const V3D &a, const V3D &b, const double tol) { for (size_t i = 0; i < 3; i++) { diff --git a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp index 7e7c5ff2bc31..d515ab3e962f 100644 --- a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp +++ b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp @@ -123,6 +123,27 @@ list calcBankRotation(PanelsSurfaceCalculator &self, const list &detPos, list no return quat; } +list transformedBoundingBoxPoints(PanelsSurfaceCalculator &self, const object &componentInfo, size_t detectorIndex, + const list &refPos, const list &rotation, const list &xaxis, const list &yaxis) { + const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); + Mantid::Kernel::Quat quatRotation{extract(rotation[0]), extract(rotation[1]), + extract(rotation[2]), extract(rotation[3])}; + const auto referencePosition = pythonListToV3D(refPos); + const auto xAxisVec = pythonListToV3D(xaxis); + const auto yAxisVec = pythonListToV3D(yaxis); + const auto boundingBoxPoints = self.transformedBoundingBoxPoints(*(cInfoSharedPtr.get()), detectorIndex, + referencePosition, quatRotation, xAxisVec, yAxisVec); + list pointA, pointB; + pointA.append(boundingBoxPoints[0].X()); + pointA.append(boundingBoxPoints[0].Y()); + pointB.append(boundingBoxPoints[1].X()); + pointB.append(boundingBoxPoints[1].Y()); + list result; + result.append(pointA); + result.append(pointB); + return result; +} + } // namespace void export_PanelsSurfaceCalculator() { @@ -149,5 +170,10 @@ void export_PanelsSurfaceCalculator() { "Finds the number of detectors in a component.") .def("calcBankRotation", &calcBankRotation, (arg("self"), arg("detPos"), arg("normal"), arg("zAxis"), arg("yAxis"), arg("samplePosition")), - "Calculates the rotation quaternion for a bank based on its position and normal vector."); + "Calculates the rotation quaternion for a bank based on its position and normal vector.") + .def("transformedBoundingBoxPoints", &transformedBoundingBoxPoints, + (arg("self"), arg("componentInfo"), arg("detectorIndex"), arg("refPos"), arg("rotation"), arg("xaxis"), + arg("yaxis")), + "Transforms a component's bounding box based on reference position and rotation. The rotation should be " + "provided as a list containing the real and imaginary parts of a quarternion (length 4)."); } diff --git a/Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py b/Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py index 7eb2b7ad1425..67dc94205d97 100644 --- a/Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py +++ b/Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py @@ -73,6 +73,17 @@ def test_calcBankRotation(self): expected_rotation = [angle, angle, 0, 0] np.testing.assert_allclose(expected_rotation, rotation, atol=1e-9) + def test_transformedBoundingBoxPoints(self): + rotation = [45, 1, 0, 0] + bounding_box_points = self._calculator.transformedBoundingBoxPoints( + self._ws.componentInfo(), 25, [0, 0, 0], rotation, [1, 0, 0], [0, 1, 0] + ) + point_a = bounding_box_points[0] + point_b = bounding_box_points[1] + tol = 1e-3 + np.testing.assert_allclose([-0.004, -0.222], point_a, atol=tol) + np.testing.assert_allclose([0.004, -0.190], point_b, atol=tol) + if __name__ == "__main__": unittest.main() diff --git a/qt/widgets/instrumentview/src/PanelsSurface.cpp b/qt/widgets/instrumentview/src/PanelsSurface.cpp index bf5e7c10713c..89530b171d77 100644 --- a/qt/widgets/instrumentview/src/PanelsSurface.cpp +++ b/qt/widgets/instrumentview/src/PanelsSurface.cpp @@ -35,22 +35,6 @@ namespace { /// static logger Mantid::Kernel::Logger g_log("PanelsSurface"); -void initialisePolygonWithTransformedBoundingBoxPoints(QPolygonF &panelPolygon, const ComponentInfo &componentInfo, - size_t detectorIndex, const V3D &refPos, const Quat &rotation, - const V3D &xaxis, const V3D &yaxis) { - auto bb = componentInfo.boundingBox(detectorIndex); - auto bbMinPoint = bb.minPoint() - refPos; - auto bbMaxPoint = bb.maxPoint() - refPos; - rotation.rotate(bbMinPoint); - rotation.rotate(bbMaxPoint); - bbMinPoint += refPos; - bbMaxPoint += refPos; - QPointF bb0(xaxis.scalar_prod(bbMinPoint), yaxis.scalar_prod(bbMinPoint)); - QPointF bb1(xaxis.scalar_prod(bbMaxPoint), yaxis.scalar_prod(bbMaxPoint)); - panelPolygon << bb0; - panelPolygon << bb1; -} - } // namespace namespace MantidQt::MantidWidgets { @@ -424,10 +408,13 @@ void PanelsSurface::processUnstructured(size_t rootIndex, std::vector &vis QVector vert; vert << p1 << p0; info->polygon = QPolygonF(vert); - // initialise bank polygon with sensible bounding box points - initialisePolygonWithTransformedBoundingBoxPoints(info->polygon, m_instrActor->componentInfo(), detectors[0], pos0, - info->rotation, m_xaxis, m_yaxis); + const auto boundingBoxPoints = this->m_calculator.transformedBoundingBoxPoints( + m_instrActor->componentInfo(), detectors[0], pos0, info->rotation, m_xaxis, m_yaxis); + QPointF bb0(boundingBoxPoints[0].X(), boundingBoxPoints[0].Y()); + QPointF bb1(boundingBoxPoints[1].X(), boundingBoxPoints[1].Y()); + info->polygon << bb0; + info->polygon << bb1; #pragma omp parallel for ordered for (int i = 0; i < static_cast(detectors.size()); ++i) { // NOLINT From 79e0d5399fed7216d9bad32db98b6f31dab0550d Mon Sep 17 00:00:00 2001 From: James Clarke Date: Tue, 22 Jul 2025 12:00:03 +0100 Subject: [PATCH 04/23] Initial code for the side by side projection --- qt/python/instrumentview/CMakeLists.txt | 1 + .../instrumentview/FullInstrumentViewModel.py | 34 +++-- .../FullInstrumentViewPresenter.py | 19 +-- .../Projections/CylindricalProjection.py | 4 +- .../instrumentview/Projections/Projection.py | 18 +-- .../instrumentview/Projections/SideBySide.py | 129 ++++++++++++++++++ .../Projections/SphericalProjection.py | 4 +- .../Projections/test/test_SideBySide.py | 20 +++ 8 files changed, 195 insertions(+), 34 deletions(-) create mode 100644 qt/python/instrumentview/instrumentview/Projections/SideBySide.py create mode 100644 qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py diff --git a/qt/python/instrumentview/CMakeLists.txt b/qt/python/instrumentview/CMakeLists.txt index a815712f4ec1..ee7c2af5aff1 100644 --- a/qt/python/instrumentview/CMakeLists.txt +++ b/qt/python/instrumentview/CMakeLists.txt @@ -12,6 +12,7 @@ set(PYTHON_TEST_FILES instrumentview/test/test_instruments.py instrumentview/Projections/test/test_projection.py instrumentview/Projections/test/test_cylindrical_projection.py + instrumentview/Projections/test/test_SideBySide.py instrumentview/Projections/test/test_spherical_projection.py ) diff --git a/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py b/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py index 3b0a6d51a87d..75cc57557829 100644 --- a/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py +++ b/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py @@ -7,7 +7,7 @@ from instrumentview.Detectors import DetectorInfo import instrumentview.Projections.SphericalProjection as iv_spherical import instrumentview.Projections.CylindricalProjection as iv_cylindrical - +import instrumentview.Projections.SideBySide as iv_side_by_side from mantid.dataobjects import Workspace2D from mantid.simpleapi import CreateDetectorTable import numpy as np @@ -16,6 +16,16 @@ class FullInstrumentViewModel: """Model for the Instrument View Window. Will calculate detector positions, indices, and integrated counts that give the colours""" + _FULL_3D = "3D" + _SPHERICAL_X = "Spherical X" + _SPHERICAL_Y = "Spherical Y" + _SPHERICAL_Z = "Spherical Z" + _CYLINDRICAL_X = "Cylindrical X" + _CYLINDRICAL_Y = "Cylindrical Y" + _CYLINDRICAL_Z = "Cylindrical Z" + _SIDE_BY_SIDE = "Side by Side" + _PROJECTION_OPTIONS = [_SPHERICAL_X, _SPHERICAL_Y, _SPHERICAL_Z, _CYLINDRICAL_X, _CYLINDRICAL_Y, _CYLINDRICAL_Z, _SIDE_BY_SIDE] + _sample_position = np.array([0, 0, 0]) _source_position = np.array([0, 0, 0]) _invalid_index = -1 @@ -179,12 +189,20 @@ def picked_detectors_info_text(self) -> list[DetectorInfo]: picked_info.append(det_info) return picked_info - def calculate_projection(self, is_spherical: bool, axis: list[int]): - """Calculate the 2D projection with the specified axis. Can be either cylindrical or spherical.""" - projection = ( - iv_spherical.SphericalProjection(self._sample_position, self._root_position, self._detector_positions, np.array(axis)) - if is_spherical - else iv_cylindrical.CylindricalProjection(self._sample_position, self._root_position, self._detector_positions, np.array(axis)) - ) + def calculate_projection(self, projection_option: str, axis: list[int]): + """Calculate the 2D projection with the specified axis. Can be cylindrical, spherical, or side-by-side.""" + if projection_option == self._SIDE_BY_SIDE: + projection = iv_side_by_side.SideBySide( + self._workspace, self._detector_ids, self._sample_position, self._root_position, self._detector_positions, np.array(axis) + ) + elif projection_option.startswith("Spherical"): + projection = iv_spherical.SphericalProjection(self._sample_position, self._root_position, self._detector_positions, np.array(axis)) + elif projection_option.startswith("Cylindrical"): + projection = iv_cylindrical.CylindricalProjection( + self._sample_position, self._root_position, self._detector_positions, np.array(axis) + ) + else: + raise ValueError(f"Unknown projection type: {projection_option}") + self._detector_projection_positions[:, :2] = projection.positions() # Assign only x and y coordinate return self._detector_projection_positions diff --git a/qt/python/instrumentview/instrumentview/FullInstrumentViewPresenter.py b/qt/python/instrumentview/instrumentview/FullInstrumentViewPresenter.py index 048a5dcb137e..d5c0881875be 100644 --- a/qt/python/instrumentview/instrumentview/FullInstrumentViewPresenter.py +++ b/qt/python/instrumentview/instrumentview/FullInstrumentViewPresenter.py @@ -18,16 +18,7 @@ class FullInstrumentViewPresenter: """Presenter for the Instrument View window""" - - _FULL_3D = "3D" - _SPHERICAL_X = "Spherical X" - _SPHERICAL_Y = "Spherical Y" - _SPHERICAL_Z = "Spherical Z" - _CYLINDRICAL_X = "Cylindrical X" - _CYLINDRICAL_Y = "Cylindrical Y" - _CYLINDRICAL_Z = "Cylindrical Z" - _PROJECTION_OPTIONS = [_FULL_3D, _SPHERICAL_X, _SPHERICAL_Y, _SPHERICAL_Z, _CYLINDRICAL_X, _CYLINDRICAL_Y, _CYLINDRICAL_Z] - + def __init__(self, view: FullInstrumentViewWindow, model: FullInstrumentViewModel): """For the given workspace, use the data from the model to plot the detectors. Also include points at the origin and any monitors.""" @@ -71,7 +62,7 @@ def projection_combo_options(self) -> tuple[int, list[str]]: default_index = possible_returns.index(default_projection) except ValueError: default_index = 0 - return default_index, self._PROJECTION_OPTIONS + return default_index, self._model._PROJECTION_OPTIONS def on_tof_limits_updated(self) -> None: """When TOF limits are changed, read the new limits and tell the presenter to update the colours accordingly""" @@ -91,7 +82,7 @@ def set_view_contour_limits(self) -> None: def on_projection_option_selected(self, selected_index: int) -> None: """Update the projection based on the selected option.""" - projection_type = self._PROJECTION_OPTIONS[selected_index] + projection_type = self._model._PROJECTION_OPTIONS[selected_index] if projection_type.startswith("3D"): # Plot orange sphere at the origin @@ -115,10 +106,12 @@ def on_projection_option_selected(self, selected_index: int) -> None: axis = [0, 1, 0] elif projection_type.endswith("Z"): axis = [0, 0, 1] + elif projection_type == self._model._SIDE_BY_SIDE: + axis = [0, 0, 1] else: raise ValueError(f"Unknown projection type {projection_type}") - self._model.calculate_projection(is_spherical, axis) + self._model.calculate_projection(projection_type, axis) self._view.disable_rectangle_picking_checkbox() self._update_view_main_plotter(self._model.detector_projection_positions, is_projection=True) diff --git a/qt/python/instrumentview/instrumentview/Projections/CylindricalProjection.py b/qt/python/instrumentview/instrumentview/Projections/CylindricalProjection.py index a5332794c037..570a6498ed4c 100644 --- a/qt/python/instrumentview/instrumentview/Projections/CylindricalProjection.py +++ b/qt/python/instrumentview/instrumentview/Projections/CylindricalProjection.py @@ -11,8 +11,8 @@ class CylindricalProjection(Projection): """2D projection with a cylindrical coordinate system, see https://en.wikipedia.org/wiki/Cylindrical_coordinate_system""" - def _calculate_2d_coordinates(self, detector_positions: np.ndarray) -> tuple[np.ndarray, np.ndarray]: - detector_relative_positions = detector_positions - self._sample_position + def _calculate_2d_coordinates(self) -> tuple[np.ndarray, np.ndarray]: + detector_relative_positions = self._detector_positions - self._sample_position z = detector_relative_positions.dot(self._projection_axis) x = detector_relative_positions.dot(self._x_axis) y = detector_relative_positions.dot(self._y_axis) diff --git a/qt/python/instrumentview/instrumentview/Projections/Projection.py b/qt/python/instrumentview/instrumentview/Projections/Projection.py index 8e4f7c2b1619..1455f7d89e85 100644 --- a/qt/python/instrumentview/instrumentview/Projections/Projection.py +++ b/qt/python/instrumentview/instrumentview/Projections/Projection.py @@ -23,11 +23,11 @@ def __init__( self._sample_position = sample_position self._root_position = root_position - self._detector_positions = detector_positions - self._projection_axis = np.array(axis) + self._detector_positions = np.array(detector_positions) + self._projection_axis = np.array(axis, dtype=np.float64) - self._x_axis = np.zeros_like(self._projection_axis) - self._y_axis = np.zeros_like(self._projection_axis) + self._x_axis = np.zeros_like(self._projection_axis, dtype=np.float64) + self._y_axis = np.zeros_like(self._projection_axis, dtype=np.float64) self._detector_x_coordinates = np.zeros(len(self._detector_positions)) self._detector_y_coordinates = np.zeros(len(self._detector_positions)) self._x_range = (0, 0) @@ -45,11 +45,11 @@ def _calculate_axes(self, root_position: np.ndarray) -> None: if z == 0 or np.abs(z) == np.linalg.norm(root_position): # Find the shortest projection of the projection axis and direct the x axis along it if np.abs(self._projection_axis[2]) < np.abs(self._projection_axis[1]): - self._x_axis = np.array([0, 0, 1]) + self._x_axis = np.array([0, 0, 1], dtype=np.float64) elif np.abs(self._projection_axis[1]) < np.abs(self._projection_axis[0]): - self._x_axis = np.array([0, 1, 0]) + self._x_axis = np.array([0, 1, 0], dtype=np.float64) else: - self._x_axis = np.array([1, 0, 0]) + self._x_axis = np.array([1, 0, 0], dtype=np.float64) else: x_axis = root_position - z * self._projection_axis self._x_axis = x_axis / np.linalg.norm(x_axis) @@ -57,13 +57,13 @@ def _calculate_axes(self, root_position: np.ndarray) -> None: self._y_axis = np.cross(self._projection_axis, self._x_axis) @abstractmethod - def _calculate_2d_coordinates(self, detector_position: np.ndarray) -> tuple[np.ndarray, np.ndarray]: + def _calculate_2d_coordinates(self) -> tuple[np.ndarray, np.ndarray]: pass def _calculate_detector_coordinates(self) -> None: """Calculate 2D projection coordinates and store data""" - self._detector_x_coordinates, self._detector_y_coordinates = self._calculate_2d_coordinates(np.array(self._detector_positions)) + self._detector_x_coordinates, self._detector_y_coordinates = self._calculate_2d_coordinates() self._x_range = (self._detector_x_coordinates.min(), self._detector_x_coordinates.max()) self._y_range = (self._detector_y_coordinates.min(), self._detector_y_coordinates.max()) diff --git a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py new file mode 100644 index 000000000000..623db44833fb --- /dev/null +++ b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py @@ -0,0 +1,129 @@ +# Mantid Repository : https://github.com/mantidproject/mantid +# +# Copyright © 2025 ISIS Rutherford Appleton Laboratory UKRI, +# NScD Oak Ridge National Laboratory, European Spallation Source, +# Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS +# SPDX - License - Identifier: GPL - 3.0 + +from instrumentview.Projections.projection import projection +import numpy as np +from instrumentview.Detectors import DetectorPosition +from mantid.api import PanelsSurfaceCalculator +from mantid.dataobjects import Workspace2D +from scipy.spatial.transform import Rotation + + +class FlatBankInfo: + """Information about a flat bank detector.""" + + rotation: Rotation + start_detector_id: int = 0 + end_detector_id: int = 0 + reference_position: list[float] + detector_id_position_map: dict[int, list[float]] = {} + dimensions: list[float] + steps: list[int] + pixels: list[int] + + def translate(self, shift: list[float]): + self.reference_position += shift + for id in self.detector_id_position_map: + self.detector_id_position_map[id] += shift + + +class SideBySide(projection): + _detector_id_to_flat_bank_map: dict[int, FlatBankInfo] = {} + _flat_banks: list[FlatBankInfo] = [] + _calculator: PanelsSurfaceCalculator + + def __init__( + self, + workspace: Workspace2D, + detector_ids: list[int] | np.ndarray, + sample_position: np.ndarray, + root_position: np.ndarray, + detector_positions: list[DetectorPosition] | np.ndarray, + axis: np.ndarray, + ): + self._calculator = PanelsSurfaceCalculator() + self._detector_ids = detector_ids + self._construct_flat_panels(workspace) + super().__init__(sample_position, root_position, detector_positions, axis) + + def _calculate_axes(self, root_position: np.ndarray) -> None: + x = [0.0] * 3 + y = [0.0] * 3 + self._calculator.setupBasisAxes(x, y, list(self._projection_axis)) + self._x_axis = x + self._y_axis = y + + def _construct_flat_panels(self, workspace: Workspace2D) -> None: + self._flat_banks.clear() + self._detector_id_to_flat_bank_map.clear() + instrument = workspace.getInstrument() + rectangular_banks = instrument.findRectDetectors() + + if len(rectangular_banks) == 0: + return + + for bank in rectangular_banks: + flat_bank = FlatBankInfo() + flat_bank.detector_id_position_map.clear() + self._flat_banks.append(flat_bank) + flat_bank.reference_position = np.array(bank.getPos()) + rotation = bank.getRotation() + flat_bank.rotation = Rotation.from_quat([rotation.real(), rotation.imagI(), rotation.imagJ(), rotation.imagK()]) + flat_bank.start_detector_id = bank.minDetectorID() + flat_bank.end_detector_id = bank.maxDetectorID() + flat_bank.dimensions = np.abs([bank.xsize(), bank.ysize(), bank.zsize()]) + flat_bank.steps = np.abs([bank.xstep(), bank.ystep(), bank.zstep()]) + flat_bank.pixels = [bank.xpixels(), bank.ypixels(), bank.zpixels()] + + self._arrange_panels() + + for bank in self._flat_banks: + y_pixels = bank.pixels[1] + x_pixels = bank.pixels[0] + origin = bank.reference_position + detector_ids = list(range(bank.start_detector_id, bank.end_detector_id + 1)) + for iy in range(y_pixels): + y_offset = iy * bank.steps[1] + for ix in range(x_pixels): + x_offset = ix * bank.steps[0] + id = detector_ids[iy * y_pixels + ix] + detector_position = np.add(origin, [x_offset, y_offset, 0]) + bank.detector_id_position_map[id] = detector_position + + for id in detector_ids: + self._detector_id_to_flat_bank_map[id] = bank + + def _arrange_panels(self) -> None: + # Going to assume all banks are roughly the same size to make the + # arrangement simpler + max_dims = np.max([b.dimensions for b in self._flat_banks], axis=0) + number_banks = len(self._flat_banks) + banks_per_row = np.ceil(np.sqrt(number_banks)) + position = [0, 0, 0] + banks_arranged = 0 + space_factor = 1.1 + for bank in self._flat_banks: + bank.reference_position = position.copy() + banks_arranged += 1 + if banks_arranged % banks_per_row == 0: + position[0] = 0 + position[1] += max_dims[1] * space_factor + else: + position[0] += bank.dimensions[0] * space_factor + + def _calculate_2d_coordinates(self) -> tuple[np.ndarray, np.ndarray]: + u_positions = [] + v_positions = [] + for id in self._detector_ids: + if id not in self._detector_id_to_flat_bank_map: + u_positions.append(0.0) + v_positions.append(0.0) + continue + bank = self._detector_id_to_flat_bank_map[id] + position = bank.detector_id_position_map[id] + u_positions.append(position[0]) + v_positions.append(position[1]) + return (np.array(u_positions), np.array(v_positions)) diff --git a/qt/python/instrumentview/instrumentview/Projections/SphericalProjection.py b/qt/python/instrumentview/instrumentview/Projections/SphericalProjection.py index 0b346f98b826..5b5ec7aed746 100644 --- a/qt/python/instrumentview/instrumentview/Projections/SphericalProjection.py +++ b/qt/python/instrumentview/instrumentview/Projections/SphericalProjection.py @@ -11,8 +11,8 @@ class SphericalProjection(Projection): """2D projection with a spherical coordinate system, see https://en.wikipedia.org/wiki/Spherical_coordinate_system""" - def _calculate_2d_coordinates(self, detector_positions: np.ndarray) -> tuple[np.ndarray, np.ndarray]: - detector_relative_positions = detector_positions - self._sample_position + def _calculate_2d_coordinates(self) -> tuple[np.ndarray, np.ndarray]: + detector_relative_positions = self._detector_positions - self._sample_position v = detector_relative_positions.dot(self._projection_axis) x = detector_relative_positions.dot(self._x_axis) y = detector_relative_positions.dot(self._y_axis) diff --git a/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py new file mode 100644 index 000000000000..883f2fed18bf --- /dev/null +++ b/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py @@ -0,0 +1,20 @@ +# Mantid Repository : https://github.com/mantidproject/mantid +# +# Copyright © 2025 ISIS Rutherford Appleton Laboratory UKRI, +# NScD Oak Ridge National Laboratory, European Spallation Source, +# Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS +# SPDX - License - Identifier: GPL - 3.0 + +import numpy as np +import unittest.mock +import unittest + + +class TestSideBySideProjection(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.root_position = np.array([0, 0, 0]) + cls.sample_position = np.array([0, 0, 0]) + cls.detector_positions = np.array([[0, 1, 0], [2, 1, 0], [-2, 1, 0]]) + + def test_calculate_2d_coordinates(self): + self.assertTrue(False) From 8484c840c50a7d7de631ee000cfa2dc5a9a488d0 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Tue, 22 Jul 2025 15:06:48 +0100 Subject: [PATCH 05/23] Use GridDetector objects instead of Rectangular All the information we need is on the GridDetector base class, so may as well use that instead of RectangularDetector. --- .../Geometry/inc/MantidGeometry/Instrument.h | 25 +++++++++++++++++++ Framework/Geometry/src/Instrument.cpp | 25 +++---------------- .../geometry/src/Exports/GridDetector.cpp | 5 ++++ .../geometry/src/Exports/Instrument.cpp | 3 ++- .../instrumentview/Projections/SideBySide.py | 2 +- 5 files changed, 37 insertions(+), 23 deletions(-) diff --git a/Framework/Geometry/inc/MantidGeometry/Instrument.h b/Framework/Geometry/inc/MantidGeometry/Instrument.h index e7a00c552b9c..3166d47df747 100644 --- a/Framework/Geometry/inc/MantidGeometry/Instrument.h +++ b/Framework/Geometry/inc/MantidGeometry/Instrument.h @@ -206,6 +206,7 @@ class MANTID_GEOMETRY_DLL Instrument : public CompAssembly { ContainsState containsRectDetectors() const; std::vector findRectDetectors() const; + std::vector findGridDetectors() const; bool isMonitorViaIndex(const size_t index) const; size_t detectorIndex(const detid_t detID) const; @@ -228,6 +229,30 @@ class MANTID_GEOMETRY_DLL Instrument : public CompAssembly { void addInstrumentChildrenToQueue(std::queue &queue) const; bool addAssemblyChildrenToQueue(std::queue &queue, IComponent_const_sptr component) const; + template std::vector> findDetectorsOfType() const { + std::queue compQueue; // Search queue + addInstrumentChildrenToQueue(compQueue); + + std::vector> detectors; + + IComponent_const_sptr comp; + + while (!compQueue.empty()) { + comp = compQueue.front(); + compQueue.pop(); + + if (!validateComponentProperties(comp)) + continue; + + if (auto const detector = std::dynamic_pointer_cast(comp)) { + detectors.push_back(detector); + } else { + // If component is a ComponentAssembly, we add its children to the queue to check if they're type T + addAssemblyChildrenToQueue(compQueue, comp); + } + } + return detectors; + } /// Private copy assignment operator Instrument &operator=(const Instrument &); diff --git a/Framework/Geometry/src/Instrument.cpp b/Framework/Geometry/src/Instrument.cpp index 833d109e2668..4341c752a26a 100644 --- a/Framework/Geometry/src/Instrument.cpp +++ b/Framework/Geometry/src/Instrument.cpp @@ -1083,28 +1083,11 @@ Instrument::ContainsState Instrument::containsRectDetectors() const { } std::vector Instrument::findRectDetectors() const { - std::queue compQueue; // Search queue - addInstrumentChildrenToQueue(compQueue); - - std::vector detectors; - - IComponent_const_sptr comp; - - while (!compQueue.empty()) { - comp = compQueue.front(); - compQueue.pop(); - - if (!validateComponentProperties(comp)) - continue; + return findDetectorsOfType(); +} - if (auto const detector = std::dynamic_pointer_cast(comp)) { - detectors.push_back(detector); - } else { - // If component is a ComponentAssembly, we add its children to the queue to check if they're Rectangular Detectors - addAssemblyChildrenToQueue(compQueue, comp); - } - } - return detectors; +std::vector Instrument::findGridDetectors() const { + return findDetectorsOfType(); } bool Instrument::validateComponentProperties(IComponent_const_sptr component) const { diff --git a/Framework/PythonInterface/mantid/geometry/src/Exports/GridDetector.cpp b/Framework/PythonInterface/mantid/geometry/src/Exports/GridDetector.cpp index 5fe6f563674b..ac9976d0f666 100644 --- a/Framework/PythonInterface/mantid/geometry/src/Exports/GridDetector.cpp +++ b/Framework/PythonInterface/mantid/geometry/src/Exports/GridDetector.cpp @@ -8,9 +8,11 @@ #include "MantidGeometry/Instrument/CompAssembly.h" #include "MantidGeometry/Instrument/GridDetectorPixel.h" #include "MantidPythonInterface/core/GetPointer.h" +#include "MantidPythonInterface/core/StlExportDefinitions.h" #include #include +using Mantid::PythonInterface::std_vector_exporter; using namespace Mantid::Geometry; using namespace boost::python; @@ -22,6 +24,9 @@ GET_POINTER_SPECIALIZATION(GridDetector) */ void export_GridDetector() { register_ptr_to_python>(); + register_ptr_to_python(); + + std_vector_exporter::wrap("std_vector_grid_detector"); class_, boost::noncopyable>("GridDetector", no_init) .def("xpixels", &GridDetector::xpixels, arg("self"), "Returns the number of pixels in the X direction") diff --git a/Framework/PythonInterface/mantid/geometry/src/Exports/Instrument.cpp b/Framework/PythonInterface/mantid/geometry/src/Exports/Instrument.cpp index a9a6cf9eee78..cac70e27d483 100644 --- a/Framework/PythonInterface/mantid/geometry/src/Exports/Instrument.cpp +++ b/Framework/PythonInterface/mantid/geometry/src/Exports/Instrument.cpp @@ -85,5 +85,6 @@ void export_Instrument() { .def("getBaseInstrument", &Instrument::baseInstrument, arg("self"), return_value_policy(), "Return reference to the base instrument") - .def("findRectDetectors", &Instrument::findRectDetectors, arg("self"), "Return a list of rectangular detectors."); + .def("findRectDetectors", &Instrument::findRectDetectors, arg("self"), "Return a list of rectangular detectors.") + .def("findGridDetectors", &Instrument::findGridDetectors, arg("self"), "Return a list of grid detectors."); } diff --git a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py index 623db44833fb..b649bd51e586 100644 --- a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py @@ -60,7 +60,7 @@ def _construct_flat_panels(self, workspace: Workspace2D) -> None: self._flat_banks.clear() self._detector_id_to_flat_bank_map.clear() instrument = workspace.getInstrument() - rectangular_banks = instrument.findRectDetectors() + rectangular_banks = instrument.findGridDetectors() if len(rectangular_banks) == 0: return From 5716a03a8258d56b77064e69f317a2e11c03aca6 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Tue, 22 Jul 2025 16:45:55 +0100 Subject: [PATCH 06/23] Fixes following rebase and tidy up --- Framework/Geometry/inc/MantidGeometry/Instrument.h | 6 ++++-- Framework/Geometry/src/Instrument.cpp | 8 -------- .../src/SpecularReflectionPositionCorrect2.cpp | 8 ++++---- .../instrumentview/Projections/SideBySide.py | 4 ++-- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/Framework/Geometry/inc/MantidGeometry/Instrument.h b/Framework/Geometry/inc/MantidGeometry/Instrument.h index 3166d47df747..6e42a519f5a6 100644 --- a/Framework/Geometry/inc/MantidGeometry/Instrument.h +++ b/Framework/Geometry/inc/MantidGeometry/Instrument.h @@ -205,8 +205,10 @@ class MANTID_GEOMETRY_DLL Instrument : public CompAssembly { /// @return Full if all detectors are rect., Partial if some, None if none ContainsState containsRectDetectors() const; - std::vector findRectDetectors() const; - std::vector findGridDetectors() const; + std::vector findRectDetectors() const { + return findDetectorsOfType(); + } + std::vector findGridDetectors() const { return findDetectorsOfType(); } bool isMonitorViaIndex(const size_t index) const; size_t detectorIndex(const detid_t detID) const; diff --git a/Framework/Geometry/src/Instrument.cpp b/Framework/Geometry/src/Instrument.cpp index 4341c752a26a..9a5a00dcdb45 100644 --- a/Framework/Geometry/src/Instrument.cpp +++ b/Framework/Geometry/src/Instrument.cpp @@ -1082,14 +1082,6 @@ Instrument::ContainsState Instrument::containsRectDetectors() const { return Instrument::ContainsState::None; } -std::vector Instrument::findRectDetectors() const { - return findDetectorsOfType(); -} - -std::vector Instrument::findGridDetectors() const { - return findDetectorsOfType(); -} - bool Instrument::validateComponentProperties(IComponent_const_sptr component) const { // Skip source, if has one if (m_sourceCache && m_sourceCache->getComponentID() == component->getComponentID()) diff --git a/Framework/Reflectometry/src/SpecularReflectionPositionCorrect2.cpp b/Framework/Reflectometry/src/SpecularReflectionPositionCorrect2.cpp index 823a2f9c4f8a..30026a36bbe3 100644 --- a/Framework/Reflectometry/src/SpecularReflectionPositionCorrect2.cpp +++ b/Framework/Reflectometry/src/SpecularReflectionPositionCorrect2.cpp @@ -18,7 +18,7 @@ using namespace Mantid::Kernel; namespace { /// Enumerations to define the rotation plane of the detector. -enum class Plane { horizontal, vertical }; +enum class RotationPlane { horizontal, vertical }; /** Return true if twoTheta increases with workspace index. * @param spectrumInfo a spectrum info @@ -199,7 +199,7 @@ void SpecularReflectionPositionCorrect2::correctDetectorPosition( const auto upAxis = referenceFrame.pointingUpAxis(); const auto thetaSignDir = referenceFrame.vecThetaSign(); const auto upDir = referenceFrame.vecPointingUp(); - const auto plane = thetaSignDir.scalar_prod(upDir) == 1. ? Plane::vertical : Plane::horizontal; + const auto plane = thetaSignDir.scalar_prod(upDir) == 1. ? RotationPlane::vertical : RotationPlane::horizontal; // Get the offset from the sample in the beam direction double beamOffset = 0.0; @@ -228,7 +228,7 @@ void SpecularReflectionPositionCorrect2::correctDetectorPosition( } moveAlg->setProperty("RelativePosition", false); moveAlg->setProperty(beamAxis, beamOffsetFromOrigin); - if (plane == Plane::vertical) { + if (plane == RotationPlane::vertical) { moveAlg->setProperty(horizontalAxis, 0.0); moveAlg->setProperty(upAxis, perpendicularOffset); } else { @@ -247,7 +247,7 @@ void SpecularReflectionPositionCorrect2::correctDetectorPosition( rotate->setProperty("ComponentName", detectorName); else rotate->setProperty("DetectorID", detectorID); - if (plane == Plane::horizontal) { + if (plane == RotationPlane::horizontal) { rotate->setProperty("X", upDir.X()); rotate->setProperty("Y", upDir.Y()); rotate->setProperty("Z", upDir.Z()); diff --git a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py index b649bd51e586..da8b84eaaf69 100644 --- a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py @@ -4,7 +4,7 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + -from instrumentview.Projections.projection import projection +from instrumentview.Projections.Projection import Projection import numpy as np from instrumentview.Detectors import DetectorPosition from mantid.api import PanelsSurfaceCalculator @@ -30,7 +30,7 @@ def translate(self, shift: list[float]): self.detector_id_position_map[id] += shift -class SideBySide(projection): +class SideBySide(Projection): _detector_id_to_flat_bank_map: dict[int, FlatBankInfo] = {} _flat_banks: list[FlatBankInfo] = [] _calculator: PanelsSurfaceCalculator From 608188c61430db44ffead071aa8a534503a1c83e Mon Sep 17 00:00:00 2001 From: James Clarke Date: Thu, 31 Jul 2025 09:30:15 +0100 Subject: [PATCH 07/23] Draw tubes that are arranged in flat banks --- .../inc/MantidAPI/PanelsSurfaceCalculator.h | 7 + Framework/API/src/PanelsSurfaceCalculator.cpp | 77 ++++++++++ .../src/Exports/PanelsSurfaceCalculator.cpp | 23 ++- .../instrumentview/Projections/SideBySide.py | 136 +++++++++++++----- 4 files changed, 210 insertions(+), 33 deletions(-) diff --git a/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h index ee15d33a177d..624b0ecbcc2a 100644 --- a/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h +++ b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h @@ -13,6 +13,8 @@ #include "MantidKernel/V2D.h" #include "MantidKernel/V3D.h" +#include + using Mantid::Geometry::ComponentInfo; using Mantid::Kernel::V3D; @@ -34,6 +36,11 @@ class MANTID_API_DLL PanelsSurfaceCalculator { size_t detectorIndex, const V3D &refPos, const Mantid::Kernel::Quat &rotation, const V3D &xaxis, const V3D &yaxis) const; + std::vector tubeDetectorParentIDs(const ComponentInfo &componentInfo, size_t rootIndex, + std::vector &visited); + std::vector> examineAllComponents( + const ComponentInfo &componentInfo, + std::function(const ComponentInfo &, size_t, std::vector &)> operation); private: Mantid::Kernel::Logger g_log = Mantid::Kernel::Logger("PanelsSurfaceCalculator"); diff --git a/Framework/API/src/PanelsSurfaceCalculator.cpp b/Framework/API/src/PanelsSurfaceCalculator.cpp index ce00fd63040b..79f336aae5af 100644 --- a/Framework/API/src/PanelsSurfaceCalculator.cpp +++ b/Framework/API/src/PanelsSurfaceCalculator.cpp @@ -229,4 +229,81 @@ PanelsSurfaceCalculator::transformedBoundingBoxPoints(const ComponentInfo &compo return {bb0, bb1}; } +std::vector> PanelsSurfaceCalculator::examineAllComponents( + const ComponentInfo &componentInfo, + std::function(const ComponentInfo &, size_t, std::vector &)> operation) { + std::vector visited(componentInfo.size(), false); + std::vector> detectorIDs; + + for (int64_t i = static_cast(componentInfo.root() - 1); i > 0; --i) { + auto children = componentInfo.children(i); + + if (children.size() > 0 && !visited[i]) { + visited[i] = true; + detectorIDs.push_back(operation(componentInfo, i, visited)); + } else if (children.size() == 0 && componentInfo.parent(i) == componentInfo.root()) { + visited[i] = true; + } + } + return detectorIDs; +} + +std::vector PanelsSurfaceCalculator::tubeDetectorParentIDs(const ComponentInfo &componentInfo, size_t rootIndex, + std::vector &visited) { + const auto componentType = componentInfo.componentType(rootIndex); + if (componentType != ComponentType::OutlineComposite) + return {}; + + const auto bankIndex0 = componentInfo.parent(rootIndex); + auto tubes = std::vector(); + bool foundFlatBank = false; + V3D normal; + size_t bankIndex; + auto addTubes = [&componentInfo](size_t parentIndex, std::vector &tubes) { + const auto &children = componentInfo.children(parentIndex); + for (auto child : children) { + if (componentInfo.componentType(child) == ComponentType::OutlineComposite) + // tube must have more than one detector to enable normal to be calculated + if (componentInfo.children(child).size() > 1) + tubes.emplace_back(child); + } + }; + + // The main use case for this method has an assembly containing a set of + // individual assemblies each of which has a single tube but together + // these tubes make a flat structure. + // Try grandparent of the tube supplied tube initially + if (componentInfo.hasParent(bankIndex0)) { + bankIndex = componentInfo.parent(bankIndex0); + auto bankChildren = &componentInfo.children(bankIndex); + + // Go down the tree to find all the tubes. + for (auto index : *bankChildren) + addTubes(index, tubes); + if (tubes.empty()) { + this->setBankVisited(componentInfo, bankIndex, visited); + return tubes; + } + // Now we found all the tubes that may form a flat struture. + // Use two of the tubes to calculate the normal to the plain of that structure + normal = tubes.size() > 1 ? this->calculateBankNormal(componentInfo, tubes) : V3D(); + // If some of the tubes are not perpendicular to the normal the structure + // isn't flat + if (!normal.nullVector() && this->isBankFlat(componentInfo, bankIndex, tubes, normal)) + foundFlatBank = true; + } + + if (!foundFlatBank) { + // Try the next level down - parent of tube supplied + tubes.clear(); + bankIndex = bankIndex0; + addTubes(bankIndex, tubes); + normal = tubes.size() > 1 ? this->calculateBankNormal(componentInfo, tubes) : V3D(); + if (normal.nullVector() || !this->isBankFlat(componentInfo, bankIndex, tubes, normal)) + this->setBankVisited(componentInfo, componentInfo.parent(rootIndex), visited); + } + + this->setBankVisited(componentInfo, bankIndex, visited); + return tubes; +} } // namespace Mantid::API diff --git a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp index d515ab3e962f..4a509bdeaf8c 100644 --- a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp +++ b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp @@ -144,6 +144,24 @@ list transformedBoundingBoxPoints(PanelsSurfaceCalculator &self, const object &c return result; } +list getAllTubeDetectorFlatGroupParents(PanelsSurfaceCalculator &self, const object &componentInfo) { + const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); + const auto allTubeGroupParents = + self.examineAllComponents(*(cInfoSharedPtr.get()), [&](const auto &cinfo, auto root, auto &visited) { + return self.tubeDetectorParentIDs(cinfo, root, visited); + }); + list pyAllTubeGroupParents; + for (size_t groupIndex = 0; groupIndex < allTubeGroupParents.size(); groupIndex++) { + const auto tubeGroupParents = allTubeGroupParents[groupIndex]; + list pyTubeGroupParents; + for (size_t tubeParentIndex = 0; tubeParentIndex < tubeGroupParents.size(); tubeParentIndex++) { + pyTubeGroupParents.append(tubeGroupParents[tubeParentIndex]); + } + pyAllTubeGroupParents.append(pyTubeGroupParents); + } + return pyAllTubeGroupParents; +} + } // namespace void export_PanelsSurfaceCalculator() { @@ -175,5 +193,8 @@ void export_PanelsSurfaceCalculator() { (arg("self"), arg("componentInfo"), arg("detectorIndex"), arg("refPos"), arg("rotation"), arg("xaxis"), arg("yaxis")), "Transforms a component's bounding box based on reference position and rotation. The rotation should be " - "provided as a list containing the real and imaginary parts of a quarternion (length 4)."); + "provided as a list containing the real and imaginary parts of a quarternion (length 4).") + .def("getAllTubeDetectorFlatGroupParents", &getAllTubeDetectorFlatGroupParents, + (arg("self"), arg("componentInfo")), + "Returns the parent component indices of detectors of all groups of tubes arranged in flat banks"); } diff --git a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py index da8b84eaaf69..99b15fb5863e 100644 --- a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py @@ -9,6 +9,7 @@ from instrumentview.Detectors import DetectorPosition from mantid.api import PanelsSurfaceCalculator from mantid.dataobjects import Workspace2D +from mantid.geometry import ComponentInfo from scipy.spatial.transform import Rotation @@ -16,24 +17,51 @@ class FlatBankInfo: """Information about a flat bank detector.""" rotation: Rotation - start_detector_id: int = 0 - end_detector_id: int = 0 - reference_position: list[float] - detector_id_position_map: dict[int, list[float]] = {} - dimensions: list[float] + detector_ids: list[int] + reference_position: np.ndarray + detector_id_position_map: dict[int, np.ndarray] = {} + dimensions: np.ndarray steps: list[int] pixels: list[int] + relative_projected_positions: np.ndarray = np.array([]) - def translate(self, shift: list[float]): + def translate(self, shift: np.ndarray): self.reference_position += shift for id in self.detector_id_position_map: self.detector_id_position_map[id] += shift + def _transform_positions_so_origin_bottom_left(self) -> None: + if not self.relative_projected_positions.any(): + return + + min_coordinates = np.min(self.relative_projected_positions, axis=0) + self.relative_projected_positions -= min_coordinates + + def calculate_projected_positions(self) -> None: + if self.relative_projected_positions.any(): + self._transform_positions_so_origin_bottom_left() + for index in range(len(self.detector_ids)): + self.detector_id_position_map[self.detector_ids[index]] = self.relative_projected_positions[index] + self.reference_position + return + + y_pixels = self.pixels[1] + x_pixels = self.pixels[0] + origin = self.reference_position + for iy in range(y_pixels): + y_offset = iy * self.steps[1] + for ix in range(x_pixels): + x_offset = ix * self.steps[0] + id = self.detector_ids[iy * y_pixels + ix] + detector_position = np.add(origin, [x_offset, y_offset, 0]) + self.detector_id_position_map[id] = detector_position + class SideBySide(Projection): _detector_id_to_flat_bank_map: dict[int, FlatBankInfo] = {} _flat_banks: list[FlatBankInfo] = [] _calculator: PanelsSurfaceCalculator + _workspace: Workspace2D + _component_index_detector_id_map: dict[int, int] def __init__( self, @@ -46,7 +74,10 @@ def __init__( ): self._calculator = PanelsSurfaceCalculator() self._detector_ids = detector_ids - self._construct_flat_panels(workspace) + self._workspace = workspace + component_info = workspace.componentInfo() + detector_component_indices = np.array(component_info.detectorsInSubtree(component_info.root())) + self._component_index_detector_id_map = dict(zip(detector_component_indices, detector_ids)) super().__init__(sample_position, root_position, detector_positions, axis) def _calculate_axes(self, root_position: np.ndarray) -> None: @@ -57,56 +88,95 @@ def _calculate_axes(self, root_position: np.ndarray) -> None: self._y_axis = y def _construct_flat_panels(self, workspace: Workspace2D) -> None: - self._flat_banks.clear() self._detector_id_to_flat_bank_map.clear() + grids = self._construct_rectangles_and_grids(workspace) + tubes = self._construct_tube_banks(workspace.componentInfo()) + self._flat_banks = grids + tubes + + if len(self._flat_banks) == 0: + return + + self._arrange_panels() + + for bank in self._flat_banks: + bank.calculate_projected_positions() + for id in bank.detector_ids: + self._detector_id_to_flat_bank_map[id] = bank + + def _construct_rectangles_and_grids(self, workspace: Workspace2D) -> list[FlatBankInfo]: instrument = workspace.getInstrument() rectangular_banks = instrument.findGridDetectors() + flat_banks = [] if len(rectangular_banks) == 0: - return + return flat_banks for bank in rectangular_banks: flat_bank = FlatBankInfo() flat_bank.detector_id_position_map.clear() - self._flat_banks.append(flat_bank) flat_bank.reference_position = np.array(bank.getPos()) rotation = bank.getRotation() flat_bank.rotation = Rotation.from_quat([rotation.real(), rotation.imagI(), rotation.imagJ(), rotation.imagK()]) - flat_bank.start_detector_id = bank.minDetectorID() - flat_bank.end_detector_id = bank.maxDetectorID() + flat_bank.detector_ids = list(range(bank.minDetectorID(), bank.maxDetectorID() + 1)) flat_bank.dimensions = np.abs([bank.xsize(), bank.ysize(), bank.zsize()]) flat_bank.steps = np.abs([bank.xstep(), bank.ystep(), bank.zstep()]) flat_bank.pixels = [bank.xpixels(), bank.ypixels(), bank.zpixels()] + flat_banks.append(flat_bank) - self._arrange_panels() + return flat_banks - for bank in self._flat_banks: - y_pixels = bank.pixels[1] - x_pixels = bank.pixels[0] - origin = bank.reference_position - detector_ids = list(range(bank.start_detector_id, bank.end_detector_id + 1)) - for iy in range(y_pixels): - y_offset = iy * bank.steps[1] - for ix in range(x_pixels): - x_offset = ix * bank.steps[0] - id = detector_ids[iy * y_pixels + ix] - detector_position = np.add(origin, [x_offset, y_offset, 0]) - bank.detector_id_position_map[id] = detector_position - - for id in detector_ids: - self._detector_id_to_flat_bank_map[id] = bank + def _construct_tube_banks(self, component_info: ComponentInfo) -> list[FlatBankInfo]: + tubeGroupParents = self._calculator.getAllTubeDetectorFlatGroupParents(component_info) + flat_banks = [] + for group in tubeGroupParents: + number_of_tube_groups = len(group) + if number_of_tube_groups == 0: + continue + detector_component_indices = np.concatenate([component_info.children(p) for p in group]) + detector_component_indices.sort() + detectors = [self._component_index_detector_id_map[i] for i in detector_component_indices] + flat_bank = FlatBankInfo() + flat_bank.detector_id_position_map.clear() + flat_bank.reference_position = np.array(component_info.position(int(detector_component_indices[0]))) + normal = np.array(self._calculator.calculateBankNormal(component_info, group)) + rotation = self._calculator.calcBankRotation( + list(flat_bank.reference_position), + list(normal), + list(self._projection_axis), + list(self._y_axis), + list(self._sample_position), + ) + flat_bank.rotation = Rotation.from_quat([rotation[0], rotation[1], rotation[2], rotation[3]]) + flat_bank.detector_ids = detectors + flat_bank.relative_projected_positions = self._calculate_projected_positions( + np.array([component_info.position(int(d)) for d in detector_component_indices]), normal, flat_bank + ) + flat_bank.dimensions = np.max(flat_bank.relative_projected_positions, axis=0) - np.min( + flat_bank.relative_projected_positions, axis=0 + ) + flat_banks.append(flat_bank) + + return flat_banks + + def _calculate_projected_positions(self, detector_positions: np.ndarray, normal: np.ndarray, flat_bank: FlatBankInfo) -> np.ndarray: + positions = detector_positions - flat_bank.reference_position + x_axis = np.array([1, 0, 0]) + if np.allclose(x_axis, normal): # if v and normal are parallel + x_axis = np.array([0, 1, 0]) + u_plane = x_axis + (x_axis.dot(normal)) * normal + u_plane = u_plane / np.linalg.norm(u_plane) + v_plane = np.cross(u_plane, normal) + return np.array([[np.dot(p, u_plane), np.dot(p, v_plane), 0] for p in positions]) def _arrange_panels(self) -> None: - # Going to assume all banks are roughly the same size to make the - # arrangement simpler max_dims = np.max([b.dimensions for b in self._flat_banks], axis=0) number_banks = len(self._flat_banks) banks_per_row = np.ceil(np.sqrt(number_banks)) - position = [0, 0, 0] + position = np.zeros(3) banks_arranged = 0 space_factor = 1.1 for bank in self._flat_banks: - bank.reference_position = position.copy() + bank.translate(position - bank.reference_position) banks_arranged += 1 if banks_arranged % banks_per_row == 0: position[0] = 0 @@ -115,6 +185,8 @@ def _arrange_panels(self) -> None: position[0] += bank.dimensions[0] * space_factor def _calculate_2d_coordinates(self) -> tuple[np.ndarray, np.ndarray]: + self._construct_flat_panels(self._workspace) + u_positions = [] v_positions = [] for id in self._detector_ids: From 23ea77ebcfb21944e3375c35e377ca9bfa4d688e Mon Sep 17 00:00:00 2001 From: James Clarke Date: Mon, 4 Aug 2025 11:58:22 +0100 Subject: [PATCH 08/23] Draw left-over detectors in a final grid. --- .../instrumentview/Projections/SideBySide.py | 62 ++++++++++++++++--- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py index 99b15fb5863e..9f1e842c1cfa 100644 --- a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py @@ -31,14 +31,14 @@ def translate(self, shift: np.ndarray): self.detector_id_position_map[id] += shift def _transform_positions_so_origin_bottom_left(self) -> None: - if not self.relative_projected_positions.any(): + if len(self.relative_projected_positions) == 0: return min_coordinates = np.min(self.relative_projected_positions, axis=0) self.relative_projected_positions -= min_coordinates def calculate_projected_positions(self) -> None: - if self.relative_projected_positions.any(): + if len(self.relative_projected_positions) > 0: self._transform_positions_so_origin_bottom_left() for index in range(len(self.detector_ids)): self.detector_id_position_map[self.detector_ids[index]] = self.relative_projected_positions[index] + self.reference_position @@ -51,7 +51,7 @@ def calculate_projected_positions(self) -> None: y_offset = iy * self.steps[1] for ix in range(x_pixels): x_offset = ix * self.steps[0] - id = self.detector_ids[iy * y_pixels + ix] + id = self.detector_ids[iy * x_pixels + ix] detector_position = np.add(origin, [x_offset, y_offset, 0]) self.detector_id_position_map[id] = detector_position @@ -80,6 +80,10 @@ def __init__( self._component_index_detector_id_map = dict(zip(detector_component_indices, detector_ids)) super().__init__(sample_position, root_position, detector_positions, axis) + def _find_and_correct_x_gap(self) -> None: + # We don't want any gaps corrected + return + def _calculate_axes(self, root_position: np.ndarray) -> None: x = [0.0] * 3 y = [0.0] * 3 @@ -92,6 +96,10 @@ def _construct_flat_panels(self, workspace: Workspace2D) -> None: grids = self._construct_rectangles_and_grids(workspace) tubes = self._construct_tube_banks(workspace.componentInfo()) self._flat_banks = grids + tubes + detectors_in_banks = np.array([d.detector_ids for d in self._flat_banks]).flatten() + remaining_detectors_bank = self._create_flat_bank_with_missing_detectors(detectors_in_banks) + if remaining_detectors_bank: + self._flat_banks.append(remaining_detectors_bank) if len(self._flat_banks) == 0: return @@ -168,13 +176,55 @@ def _calculate_projected_positions(self, detector_positions: np.ndarray, normal: v_plane = np.cross(u_plane, normal) return np.array([[np.dot(p, u_plane), np.dot(p, v_plane), 0] for p in positions]) + def _create_flat_bank_with_missing_detectors(self, detectors_already_in_banks: np.ndarray) -> FlatBankInfo: + detectors = np.array(self._detector_ids) + missing_detectors = np.setdiff1d(detectors, detectors_already_in_banks) + number_of_detectors = len(missing_detectors) + if number_of_detectors == 0: + return None + flat_bank = FlatBankInfo() + flat_bank.detector_id_position_map.clear() + flat_bank.reference_position = np.zeros(3) + flat_bank.rotation = Rotation.identity() + flat_bank.detector_ids = list(missing_detectors) + detectors_on_x_axis = np.ceil(np.sqrt(number_of_detectors)).astype(int) + # Base separation roughly on any existing detector densities + separation = 0.01 + if len(self._flat_banks) > 0 and detectors_on_x_axis > 1: + existing_max_dims = np.max([b.dimensions for b in self._flat_banks], axis=0) + area_per_detector = (existing_max_dims[0] * existing_max_dims[1]) / sum([len(f.detector_ids) for f in self._flat_banks]) + if area_per_detector > 0: + separation = max([np.sqrt(area_per_detector) / number_of_detectors / (detectors_on_x_axis - 1), 0.001]) + + # Arrange detectors into a grid with sqrt(len) points on the x axis, the last + # row may have less than that number + points_in_square_grid = ((np.floor(number_of_detectors / detectors_on_x_axis)) * detectors_on_x_axis).astype(int) + square_grid = missing_detectors[0:points_in_square_grid].reshape((detectors_on_x_axis, -1)) + # Get position for each detector in square grid + row_indices, column_indices = np.indices(square_grid.shape) + square_grid_positions = np.stack((row_indices, column_indices, np.zeros_like(row_indices)), axis=2) * separation + flat_bank.relative_projected_positions = square_grid_positions.reshape((-1, 3)) + + # Do the same for the remainder. Unless we're lucky, we'll have a partial row of + # detectors left over that we need to arrange + if points_in_square_grid < number_of_detectors: + remainder = missing_detectors[points_in_square_grid:] + row_y = square_grid.shape[1] * separation + remainder_positions = [[x * separation, row_y, 0] for x in range(len(remainder))] + flat_bank.relative_projected_positions = np.append(flat_bank.relative_projected_positions, remainder_positions, axis=0) + + flat_bank.dimensions = np.max(flat_bank.relative_projected_positions, axis=0) - np.min( + flat_bank.relative_projected_positions, axis=0 + ) + return flat_bank + def _arrange_panels(self) -> None: max_dims = np.max([b.dimensions for b in self._flat_banks], axis=0) number_banks = len(self._flat_banks) banks_per_row = np.ceil(np.sqrt(number_banks)) position = np.zeros(3) banks_arranged = 0 - space_factor = 1.1 + space_factor = 1.2 for bank in self._flat_banks: bank.translate(position - bank.reference_position) banks_arranged += 1 @@ -191,9 +241,7 @@ def _calculate_2d_coordinates(self) -> tuple[np.ndarray, np.ndarray]: v_positions = [] for id in self._detector_ids: if id not in self._detector_id_to_flat_bank_map: - u_positions.append(0.0) - v_positions.append(0.0) - continue + raise RuntimeError(f"Detector with ID {id} not found in projection") bank = self._detector_id_to_flat_bank_map[id] position = bank.detector_id_position_map[id] u_positions.append(position[0]) From 4dfeb92f5e1cab07cbebe9796439bdce5b7e6c78 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Mon, 4 Aug 2025 14:39:08 +0100 Subject: [PATCH 09/23] Interactive scalar bars, vertical, on the right --- .../instrumentview/instrumentview/FullInstrumentViewWindow.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qt/python/instrumentview/instrumentview/FullInstrumentViewWindow.py b/qt/python/instrumentview/instrumentview/FullInstrumentViewWindow.py index 74d6f32ac679..293dade913e1 100644 --- a/qt/python/instrumentview/instrumentview/FullInstrumentViewWindow.py +++ b/qt/python/instrumentview/instrumentview/FullInstrumentViewWindow.py @@ -316,7 +316,8 @@ def add_simple_shape(self, mesh: PolyData, colour=None, pickable=False) -> None: def add_main_mesh(self, mesh: PolyData, is_projection: bool, scalars=None) -> None: """Draw the given mesh in the main plotter window""" self.main_plotter.clear() - self.main_plotter.add_mesh(mesh, pickable=False, scalars=scalars, render_points_as_spheres=True, point_size=15) + scalar_bar_args = dict(interactive=True, vertical=True) if scalars is not None else None + self.main_plotter.add_mesh(mesh, pickable=False, scalars=scalars, render_points_as_spheres=True, point_size=15, scalar_bar_args=scalar_bar_args) if not self.main_plotter.off_screen: self.main_plotter.enable_trackball_style() From 4594407be6937742c64251642d96e7b5988a16ac Mon Sep 17 00:00:00 2001 From: James Clarke Date: Thu, 7 Aug 2025 16:25:49 +0100 Subject: [PATCH 10/23] Read bank override positions from IDF It's possible to specify a bank's position in the side-by-side view in the IDF. If that's happened, then put the panel in that position. --- .../inc/MantidAPI/PanelsSurfaceCalculator.h | 7 +++++ Framework/API/src/PanelsSurfaceCalculator.cpp | 9 +++++++ .../src/Exports/PanelsSurfaceCalculator.cpp | 26 ++++++++++++++++++- .../instrumentview/Projections/SideBySide.py | 16 +++++++++++- .../instrumentview/src/PanelsSurface.cpp | 13 +++------- 5 files changed, 60 insertions(+), 11 deletions(-) diff --git a/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h index 624b0ecbcc2a..b37750a8fce0 100644 --- a/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h +++ b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h @@ -7,6 +7,7 @@ #pragma once #include "MantidAPI/DllConfig.h" +#include "MantidGeometry/Instrument.h" #include "MantidGeometry/Instrument/ComponentInfo.h" #include "MantidKernel/Logger.h" #include "MantidKernel/Quat.h" @@ -14,8 +15,11 @@ #include "MantidKernel/V3D.h" #include +#include using Mantid::Geometry::ComponentInfo; +using Mantid::Geometry::Instrument; +using Mantid::Geometry::Instrument_const_sptr; using Mantid::Kernel::V3D; namespace Mantid { @@ -41,6 +45,9 @@ class MANTID_API_DLL PanelsSurfaceCalculator { std::vector> examineAllComponents( const ComponentInfo &componentInfo, std::function(const ComponentInfo &, size_t, std::vector &)> operation); + std::optional getSideBySideViewPos(const ComponentInfo &componentInfo, + const Instrument_const_sptr instrument, + const size_t componentIndex) const; private: Mantid::Kernel::Logger g_log = Mantid::Kernel::Logger("PanelsSurfaceCalculator"); diff --git a/Framework/API/src/PanelsSurfaceCalculator.cpp b/Framework/API/src/PanelsSurfaceCalculator.cpp index 79f336aae5af..df025f37cc6a 100644 --- a/Framework/API/src/PanelsSurfaceCalculator.cpp +++ b/Framework/API/src/PanelsSurfaceCalculator.cpp @@ -306,4 +306,13 @@ std::vector PanelsSurfaceCalculator::tubeDetectorParentIDs(const Compone this->setBankVisited(componentInfo, bankIndex, visited); return tubes; } + +std::optional PanelsSurfaceCalculator::getSideBySideViewPos(const ComponentInfo &componentInfo, + const Instrument_const_sptr instrument, + const size_t componentIndex) const { + const auto *componentID = componentInfo.componentID(componentIndex); + const auto component = instrument->getComponentByID(componentID); + return component->getSideBySideViewPos(); +} + } // namespace Mantid::API diff --git a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp index 4a509bdeaf8c..a4934ef61dd4 100644 --- a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp +++ b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp @@ -162,6 +162,26 @@ list getAllTubeDetectorFlatGroupParents(PanelsSurfaceCalculator &self, const obj return pyAllTubeGroupParents; } +tuple getSideBySideViewPos(PanelsSurfaceCalculator &self, const object &componentInfo, const object &instrument, + const size_t componentIndex) { + const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); + const std::shared_ptr instrumentSharedPtr = extract>(instrument); + const auto sideBySidePos = self.getSideBySideViewPos(*(cInfoSharedPtr).get(), instrumentSharedPtr, componentIndex); + list position; + list result; + if (!sideBySidePos.has_value()) { + position.append(0); + position.append(0); + result.append(false); + } else { + position.append(sideBySidePos->X()); + position.append(sideBySidePos->Y()); + result.append(true); + } + result.append(position); + return tuple(result); +} + } // namespace void export_PanelsSurfaceCalculator() { @@ -196,5 +216,9 @@ void export_PanelsSurfaceCalculator() { "provided as a list containing the real and imaginary parts of a quarternion (length 4).") .def("getAllTubeDetectorFlatGroupParents", &getAllTubeDetectorFlatGroupParents, (arg("self"), arg("componentInfo")), - "Returns the parent component indices of detectors of all groups of tubes arranged in flat banks"); + "Returns the parent component indices of detectors of all groups of tubes arranged in flat banks") + .def("getSideBySideViewPos", &getSideBySideViewPos, + (arg("self"), arg("componentInfo"), arg("instrument"), arg("componentIndex")), + "Returns a tuple indicating whether the bank side-by-side projection position has been specified in the " + "IDF, and what it is."); } diff --git a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py index 9f1e842c1cfa..6706f37c851c 100644 --- a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py @@ -24,6 +24,7 @@ class FlatBankInfo: steps: list[int] pixels: list[int] relative_projected_positions: np.ndarray = np.array([]) + has_position_in_idf: bool = False def translate(self, shift: np.ndarray): self.reference_position += shift @@ -62,6 +63,7 @@ class SideBySide(Projection): _calculator: PanelsSurfaceCalculator _workspace: Workspace2D _component_index_detector_id_map: dict[int, int] + _detector_id_component_index_map: dict[int, int] def __init__( self, @@ -78,6 +80,7 @@ def __init__( component_info = workspace.componentInfo() detector_component_indices = np.array(component_info.detectorsInSubtree(component_info.root())) self._component_index_detector_id_map = dict(zip(detector_component_indices, detector_ids)) + self._detector_id_component_index_map = {id: c for c, id in self._component_index_detector_id_map.items()} super().__init__(sample_position, root_position, detector_positions, axis) def _find_and_correct_x_gap(self) -> None: @@ -114,6 +117,7 @@ def _construct_flat_panels(self, workspace: Workspace2D) -> None: def _construct_rectangles_and_grids(self, workspace: Workspace2D) -> list[FlatBankInfo]: instrument = workspace.getInstrument() rectangular_banks = instrument.findGridDetectors() + component_info = self._workspace.componentInfo() flat_banks = [] if len(rectangular_banks) == 0: @@ -129,6 +133,11 @@ def _construct_rectangles_and_grids(self, workspace: Workspace2D) -> list[FlatBa flat_bank.dimensions = np.abs([bank.xsize(), bank.ysize(), bank.zsize()]) flat_bank.steps = np.abs([bank.xstep(), bank.ystep(), bank.zstep()]) flat_bank.pixels = [bank.xpixels(), bank.ypixels(), bank.zpixels()] + parent_component_index = component_info.parent(int(self._detector_id_component_index_map[flat_bank.detector_ids[0]])) + override_pos = self._calculator.getSideBySideViewPos(component_info, instrument, parent_component_index) + flat_bank.has_position_in_idf = override_pos[0] + if flat_bank.has_position_in_idf: + flat_bank.reference_position = np.array(override_pos[1] + [0]) flat_banks.append(flat_bank) return flat_banks @@ -162,6 +171,10 @@ def _construct_tube_banks(self, component_info: ComponentInfo) -> list[FlatBankI flat_bank.dimensions = np.max(flat_bank.relative_projected_positions, axis=0) - np.min( flat_bank.relative_projected_positions, axis=0 ) + override_pos = self._calculator.getSideBySideViewPos(component_info, self._workspace.getInstrument(), group[0]) + flat_bank.has_position_in_idf = override_pos[0] + if flat_bank.has_position_in_idf: + flat_bank.reference_position = np.array(override_pos[1] + [0]) flat_banks.append(flat_bank) return flat_banks @@ -226,7 +239,8 @@ def _arrange_panels(self) -> None: banks_arranged = 0 space_factor = 1.2 for bank in self._flat_banks: - bank.translate(position - bank.reference_position) + if not bank.has_position_in_idf: + bank.translate(position - bank.reference_position) banks_arranged += 1 if banks_arranged % banks_per_row == 0: position[0] = 0 diff --git a/qt/widgets/instrumentview/src/PanelsSurface.cpp b/qt/widgets/instrumentview/src/PanelsSurface.cpp index 89530b171d77..5b563204b5e6 100644 --- a/qt/widgets/instrumentview/src/PanelsSurface.cpp +++ b/qt/widgets/instrumentview/src/PanelsSurface.cpp @@ -215,9 +215,7 @@ void PanelsSurface::processStructured(size_t rootIndex) { } } - auto compID = componentInfo.componentID(rootIndex); - auto component = m_instrActor->getInstrument()->getComponentByID(compID); - info->bankCentreOverride = component->getSideBySideViewPos(); + info->bankCentreOverride = m_calculator.getSideBySideViewPos(componentInfo, m_instrActor->getInstrument(), rootIndex); } void PanelsSurface::processGrid(size_t rootIndex) { @@ -338,9 +336,7 @@ std::optional PanelsSurface::processTubes(size_t rootIndex) { // read any bank centre override from the bank - note that due to the logic higher up that sets bankIndex, the // override will only be read from components one or two levels up from a tube - auto compID = componentInfo.componentID(bankIndex); - auto component = m_instrActor->getInstrument()->getComponentByID(compID); - info->bankCentreOverride = component->getSideBySideViewPos(); + info->bankCentreOverride = m_calculator.getSideBySideViewPos(componentInfo, m_instrActor->getInstrument(), bankIndex); return bankIndex; } @@ -425,9 +421,8 @@ void PanelsSurface::processUnstructured(size_t rootIndex, std::vector &vis info->polygon << QPointF(udet.u, udet.v); } - auto compID = componentInfo.componentID(rootIndex); - auto component = m_instrActor->getInstrument()->getComponentByID(compID); - info->bankCentreOverride = component->getSideBySideViewPos(); + info->bankCentreOverride = + m_calculator.getSideBySideViewPos(componentInfo, m_instrActor->getInstrument(), rootIndex); } } From 4903d76ce13431e2ce726fe03d06b868f2f19efa Mon Sep 17 00:00:00 2001 From: James Clarke Date: Tue, 16 Sep 2025 13:50:02 +0100 Subject: [PATCH 11/23] Fixes following rebase --- .../instrumentview/FullInstrumentViewModel.py | 15 +++++++++++++-- .../instrumentview/FullInstrumentViewPresenter.py | 12 ++---------- .../instrumentview/test/test_model.py | 8 ++++---- .../instrumentview/test/test_view.py | 7 ++++++- 4 files changed, 25 insertions(+), 17 deletions(-) diff --git a/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py b/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py index 75cc57557829..b29c18e280ca 100644 --- a/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py +++ b/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py @@ -24,7 +24,16 @@ class FullInstrumentViewModel: _CYLINDRICAL_Y = "Cylindrical Y" _CYLINDRICAL_Z = "Cylindrical Z" _SIDE_BY_SIDE = "Side by Side" - _PROJECTION_OPTIONS = [_SPHERICAL_X, _SPHERICAL_Y, _SPHERICAL_Z, _CYLINDRICAL_X, _CYLINDRICAL_Y, _CYLINDRICAL_Z, _SIDE_BY_SIDE] + _PROJECTION_OPTIONS = [ + _FULL_3D, + _SPHERICAL_X, + _SPHERICAL_Y, + _SPHERICAL_Z, + _CYLINDRICAL_X, + _CYLINDRICAL_Y, + _CYLINDRICAL_Z, + _SIDE_BY_SIDE, + ] _sample_position = np.array([0, 0, 0]) _source_position = np.array([0, 0, 0]) @@ -196,7 +205,9 @@ def calculate_projection(self, projection_option: str, axis: list[int]): self._workspace, self._detector_ids, self._sample_position, self._root_position, self._detector_positions, np.array(axis) ) elif projection_option.startswith("Spherical"): - projection = iv_spherical.SphericalProjection(self._sample_position, self._root_position, self._detector_positions, np.array(axis)) + projection = iv_spherical.SphericalProjection( + self._sample_position, self._root_position, self._detector_positions, np.array(axis) + ) elif projection_option.startswith("Cylindrical"): projection = iv_cylindrical.CylindricalProjection( self._sample_position, self._root_position, self._detector_positions, np.array(axis) diff --git a/qt/python/instrumentview/instrumentview/FullInstrumentViewPresenter.py b/qt/python/instrumentview/instrumentview/FullInstrumentViewPresenter.py index d5c0881875be..e41d3c463d6a 100644 --- a/qt/python/instrumentview/instrumentview/FullInstrumentViewPresenter.py +++ b/qt/python/instrumentview/instrumentview/FullInstrumentViewPresenter.py @@ -18,7 +18,7 @@ class FullInstrumentViewPresenter: """Presenter for the Instrument View window""" - + def __init__(self, view: FullInstrumentViewWindow, model: FullInstrumentViewModel): """For the given workspace, use the data from the model to plot the detectors. Also include points at the origin and any monitors.""" @@ -58,7 +58,7 @@ def setup(self): def projection_combo_options(self) -> tuple[int, list[str]]: default_projection = self._model.default_projection try: - possible_returns = ["3D", "SPHERICAL_X", "SPHERICAL_Y", "SPHERICAL_Z", "CYLINDRICAL_X", "CYLINDRICAL_Y", "CYLINDRICAL_Z"] + possible_returns = self._model._PROJECTION_OPTIONS default_index = possible_returns.index(default_projection) except ValueError: default_index = 0 @@ -92,14 +92,6 @@ def on_projection_option_selected(self, selected_index: int) -> None: self._update_view_main_plotter(self._model.detector_positions, is_projection=False) return - is_spherical = True - if projection_type.startswith("Spherical"): - is_spherical = True - elif projection_type.startswith("Cylindrical"): - is_spherical = False - else: - raise ValueError(f"Unknown projection type: {projection_type}") - if projection_type.endswith("X"): axis = [1, 0, 0] elif projection_type.endswith("Y"): diff --git a/qt/python/instrumentview/instrumentview/test/test_model.py b/qt/python/instrumentview/instrumentview/test/test_model.py index 33930dfa392d..77b3c34ed863 100644 --- a/qt/python/instrumentview/instrumentview/test/test_model.py +++ b/qt/python/instrumentview/instrumentview/test/test_model.py @@ -125,13 +125,13 @@ def test_detectors_with_no_spectra(self): @mock.patch("instrumentview.Projections.SphericalProjection.SphericalProjection") def test_calculate_spherical_projection(self, mock_spherical_projection): - self._run_projection_test(mock_spherical_projection, True) + self._run_projection_test(mock_spherical_projection, FullInstrumentViewModel._SPHERICAL_Y) @mock.patch("instrumentview.Projections.CylindricalProjection.CylindricalProjection") def test_calculate_cylindrical_projection(self, mock_cylindrical_projection): - self._run_projection_test(mock_cylindrical_projection, False) + self._run_projection_test(mock_cylindrical_projection, FullInstrumentViewModel._CYLINDRICAL_Y) - def _run_projection_test(self, mock_projection_constructor, is_spherical): + def _run_projection_test(self, mock_projection_constructor, projection_option): self._mock_detector_table([1, 2, 3]) mock_workspace = self._create_mock_workspace([1, 2, 3]) model = FullInstrumentViewModel(mock_workspace) @@ -139,7 +139,7 @@ def _run_projection_test(self, mock_projection_constructor, is_spherical): mock_projection = mock.MagicMock() mock_projection.positions.return_value = [[1, 2], [1, 2], [1, 2]] mock_projection_constructor.return_value = mock_projection - points = model.calculate_projection(is_spherical, axis=[0, 1, 0]) + points = model.calculate_projection(projection_option, axis=[0, 1, 0]) mock_projection_constructor.assert_called_once() self.assertTrue(all(all(point == [1, 2, 0]) for point in points)) diff --git a/qt/python/instrumentview/instrumentview/test/test_view.py b/qt/python/instrumentview/instrumentview/test/test_view.py index be261da39706..942f828e6152 100644 --- a/qt/python/instrumentview/instrumentview/test/test_view.py +++ b/qt/python/instrumentview/instrumentview/test/test_view.py @@ -63,7 +63,12 @@ def test_add_main_mesh(self): mock_scalars = MagicMock() self._view.add_main_mesh(mock_mesh, False, mock_scalars) self._view.main_plotter.add_mesh.assert_called_once_with( - mock_mesh, pickable=False, scalars=mock_scalars, render_points_as_spheres=True, point_size=15 + mock_mesh, + pickable=False, + scalars=mock_scalars, + render_points_as_spheres=True, + point_size=15, + scalar_bar_args={"interactive": True, "vertical": True}, ) def test_add_pickable_main_mesh(self): From d8b764ea94317e8e956cc65771c28a0cd0f8d58c Mon Sep 17 00:00:00 2001 From: James Clarke Date: Tue, 16 Sep 2025 13:53:08 +0100 Subject: [PATCH 12/23] Disable logging for CreateDetectorTable --- .../instrumentview/instrumentview/FullInstrumentViewModel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py b/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py index b29c18e280ca..65252363c2a1 100644 --- a/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py +++ b/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py @@ -53,7 +53,7 @@ def setup(self): self._source_position = np.array(component_info.sourcePosition()) if has_source else np.array([0, 0, 0]) self._root_position = np.array(component_info.position(0)) - detector_info_table = CreateDetectorTable(self._workspace, IncludeDetectorPosition=True, StoreInADS=False) + detector_info_table = CreateDetectorTable(self._workspace, IncludeDetectorPosition=True, StoreInADS=False, EnableLogging=False) # Might have comma-separated multiple detectors, choose first one in the string in that case first_numbers = np.char.split(detector_info_table.column("Detector ID(s)"), sep=",") From 79270546afb3ac7643048a87205e5cfaa49c5022 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Tue, 16 Sep 2025 14:23:38 +0100 Subject: [PATCH 13/23] Shrink text labels on scalar bar --- .../instrumentview/FullInstrumentViewWindow.py | 6 ++++-- qt/python/instrumentview/instrumentview/test/test_view.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/qt/python/instrumentview/instrumentview/FullInstrumentViewWindow.py b/qt/python/instrumentview/instrumentview/FullInstrumentViewWindow.py index 293dade913e1..b17ac20a9b17 100644 --- a/qt/python/instrumentview/instrumentview/FullInstrumentViewWindow.py +++ b/qt/python/instrumentview/instrumentview/FullInstrumentViewWindow.py @@ -316,8 +316,10 @@ def add_simple_shape(self, mesh: PolyData, colour=None, pickable=False) -> None: def add_main_mesh(self, mesh: PolyData, is_projection: bool, scalars=None) -> None: """Draw the given mesh in the main plotter window""" self.main_plotter.clear() - scalar_bar_args = dict(interactive=True, vertical=True) if scalars is not None else None - self.main_plotter.add_mesh(mesh, pickable=False, scalars=scalars, render_points_as_spheres=True, point_size=15, scalar_bar_args=scalar_bar_args) + scalar_bar_args = dict(interactive=True, vertical=True, title_font_size=15, label_font_size=12) if scalars is not None else None + self.main_plotter.add_mesh( + mesh, pickable=False, scalars=scalars, render_points_as_spheres=True, point_size=15, scalar_bar_args=scalar_bar_args + ) if not self.main_plotter.off_screen: self.main_plotter.enable_trackball_style() diff --git a/qt/python/instrumentview/instrumentview/test/test_view.py b/qt/python/instrumentview/instrumentview/test/test_view.py index 942f828e6152..c09356cd70b3 100644 --- a/qt/python/instrumentview/instrumentview/test/test_view.py +++ b/qt/python/instrumentview/instrumentview/test/test_view.py @@ -68,7 +68,7 @@ def test_add_main_mesh(self): scalars=mock_scalars, render_points_as_spheres=True, point_size=15, - scalar_bar_args={"interactive": True, "vertical": True}, + scalar_bar_args={"interactive": True, "vertical": True, "title_font_size": 15, "label_font_size": 12}, ) def test_add_pickable_main_mesh(self): From 1d902ea246a1bc5a1f330992a9ac069500c84e98 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Wed, 17 Sep 2025 16:21:50 +0100 Subject: [PATCH 14/23] Only use positions of valid detectors for projections --- .../instrumentview/FullInstrumentViewModel.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py b/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py index 65252363c2a1..97ae678a3a7c 100644 --- a/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py +++ b/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py @@ -70,7 +70,7 @@ def setup(self): # Initialise with zeros self._counts = np.zeros_like(self._detector_ids) self._counts_limits = (0, 0) - self._detector_projection_positions = np.zeros_like(self._detector_positions) + self._detector_projection_positions = np.zeros_like(self.detector_positions) self._detector_is_picked = np.full(len(self._detector_ids[self._is_valid]), False) # Get min and max tof values @@ -106,7 +106,7 @@ def detector_positions(self) -> np.ndarray: @property def detector_projection_positions(self) -> np.ndarray: - return self._detector_projection_positions[self._is_valid] + return self._detector_projection_positions @property def detector_ids(self) -> np.ndarray: @@ -202,15 +202,15 @@ def calculate_projection(self, projection_option: str, axis: list[int]): """Calculate the 2D projection with the specified axis. Can be cylindrical, spherical, or side-by-side.""" if projection_option == self._SIDE_BY_SIDE: projection = iv_side_by_side.SideBySide( - self._workspace, self._detector_ids, self._sample_position, self._root_position, self._detector_positions, np.array(axis) + self._workspace, self.detector_ids, self.sample_position, self._root_position, self.detector_positions, np.array(axis) ) elif projection_option.startswith("Spherical"): projection = iv_spherical.SphericalProjection( - self._sample_position, self._root_position, self._detector_positions, np.array(axis) + self.sample_position, self._root_position, self.detector_positions, np.array(axis) ) elif projection_option.startswith("Cylindrical"): projection = iv_cylindrical.CylindricalProjection( - self._sample_position, self._root_position, self._detector_positions, np.array(axis) + self.sample_position, self._root_position, self.detector_positions, np.array(axis) ) else: raise ValueError(f"Unknown projection type: {projection_option}") From e034c35ebcca1d01c3c6303a1eee51421fd0836b Mon Sep 17 00:00:00 2001 From: James Clarke Date: Thu, 18 Sep 2025 10:56:27 +0100 Subject: [PATCH 15/23] Handle instruments with only a subset of detectors At least one instrument, WISH, has multiple detector IDs at the same position. We only plot one of those, currently the first, but the side-by-side projection needs to get the relevant component indices for the given detectors. This is the index of the given detectors in the list of all the detectors. --- .../instrumentview/Projections/SideBySide.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py index 6706f37c851c..f92035feb258 100644 --- a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py @@ -77,8 +77,11 @@ def __init__( self._calculator = PanelsSurfaceCalculator() self._detector_ids = detector_ids self._workspace = workspace - component_info = workspace.componentInfo() - detector_component_indices = np.array(component_info.detectorsInSubtree(component_info.root())) + all_detector_ids = np.array(workspace.detectorInfo().detectorIDs()) + # We can have a subset of all detector IDs, so we need to know the index of the given detectors + # in the list of all the detectors, since this will give us the component index, which we + # will need later. If there are N detectors, then 0,..., N-1 are their component indices + detector_component_indices = np.where(np.isin(all_detector_ids, detector_ids))[0] self._component_index_detector_id_map = dict(zip(detector_component_indices, detector_ids)) self._detector_id_component_index_map = {id: c for c, id in self._component_index_detector_id_map.items()} super().__init__(sample_position, root_position, detector_positions, axis) @@ -151,7 +154,9 @@ def _construct_tube_banks(self, component_info: ComponentInfo) -> list[FlatBankI continue detector_component_indices = np.concatenate([component_info.children(p) for p in group]) detector_component_indices.sort() - detectors = [self._component_index_detector_id_map[i] for i in detector_component_indices] + detectors = [ + self._component_index_detector_id_map[i] for i in detector_component_indices if i in self._component_index_detector_id_map + ] flat_bank = FlatBankInfo() flat_bank.detector_id_position_map.clear() flat_bank.reference_position = np.array(component_info.position(int(detector_component_indices[0]))) From 7fcfe3590f8dd4f9c31ca8cf522980df3d52eecc Mon Sep 17 00:00:00 2001 From: James Clarke Date: Thu, 18 Sep 2025 12:07:06 +0100 Subject: [PATCH 16/23] Fix cppcheck warnings --- .../Geometry/inc/MantidGeometry/Instrument.h | 7 ++----- Framework/Geometry/src/Instrument.cpp | 5 ----- .../src/Exports/PanelsSurfaceCalculator.cpp | 19 ++++++++++--------- .../CMake/CppCheck_Suppressions.txt.in | 1 - 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/Framework/Geometry/inc/MantidGeometry/Instrument.h b/Framework/Geometry/inc/MantidGeometry/Instrument.h index 6e42a519f5a6..7214ff9806b9 100644 --- a/Framework/Geometry/inc/MantidGeometry/Instrument.h +++ b/Framework/Geometry/inc/MantidGeometry/Instrument.h @@ -216,9 +216,6 @@ class MANTID_GEOMETRY_DLL Instrument : public CompAssembly { bool isEmptyInstrument() const; - /// Add a component to the instrument - virtual int add(IComponent *component) override; - void parseTreeAndCacheBeamline(); std::pair, std::unique_ptr> makeBeamline(ParameterMap &pmap, const ParameterMap *source = nullptr) const; @@ -271,11 +268,11 @@ class MANTID_GEOMETRY_DLL Instrument : public CompAssembly { /// Purpose to hold copy of source component. For now assumed to be just one /// component - const IComponent *m_sourceCache; + const IComponent *m_sourceCache = nullptr; /// Purpose to hold copy of samplePos component. For now assumed to be just /// one component - const IComponent *m_sampleCache; + const IComponent *m_sampleCache = nullptr; /// To store info about the parameters defined in IDF. Indexed according to /// logfile-IDs, which equals logfile filename minus the run number and file diff --git a/Framework/Geometry/src/Instrument.cpp b/Framework/Geometry/src/Instrument.cpp index 9a5a00dcdb45..514e395ab823 100644 --- a/Framework/Geometry/src/Instrument.cpp +++ b/Framework/Geometry/src/Instrument.cpp @@ -1133,11 +1133,6 @@ bool Instrument::isMonitorViaIndex(const size_t index) const { bool Instrument::isEmptyInstrument() const { return this->nelements() == 0; } -int Instrument::add(IComponent *component) { - // invalidate cache - return CompAssembly::add(component); -} - /// Returns the index for a detector ID. Used for accessing DetectorInfo. size_t Instrument::detectorIndex(const detid_t detID) const { const auto &baseInstr = m_map ? *m_instr : *this; diff --git a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp index a4934ef61dd4..c1021fa15066 100644 --- a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp +++ b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp @@ -25,7 +25,7 @@ V3D pythonListToV3D(const object &pyList) { return {extract(pyList[0]), extract(pyList[1]), extract(pyList[2])}; } -void setupBasisAxes(PanelsSurfaceCalculator &self, list &xAxis, list &yAxis, const list &zAxis) { +void setupBasisAxes(const PanelsSurfaceCalculator &self, list &xAxis, list &yAxis, const list &zAxis) { auto x = pythonListToV3D(xAxis); auto y = pythonListToV3D(yAxis); const auto z = pythonListToV3D(zAxis); @@ -36,7 +36,7 @@ void setupBasisAxes(PanelsSurfaceCalculator &self, list &xAxis, list &yAxis, con } } -list retrievePanelCorners(PanelsSurfaceCalculator &self, const object &componentInfo, const size_t rootIndex) { +list retrievePanelCorners(const PanelsSurfaceCalculator &self, const object &componentInfo, const size_t rootIndex) { const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); const auto panelCorners = self.retrievePanelCorners(*(cInfoSharedPtr.get()), rootIndex); list panelCornersList; @@ -47,7 +47,7 @@ list retrievePanelCorners(PanelsSurfaceCalculator &self, const object &component return panelCornersList; } -list calculatePanelNormal(PanelsSurfaceCalculator &self, const list &panelCorners) { +list calculatePanelNormal(const PanelsSurfaceCalculator &self, const list &panelCorners) { if (len(panelCorners) != 4) { throw std::invalid_argument("Must be 4 panel corners"); } @@ -83,7 +83,7 @@ list calculateBankNormal(PanelsSurfaceCalculator &self, const object &componentI return normalList; } -void setBankVisited(PanelsSurfaceCalculator &self, const object &componentInfo, const size_t bankIndex, +void setBankVisited(const PanelsSurfaceCalculator &self, const object &componentInfo, const size_t bankIndex, list &visitedComponents) { const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); std::vector visitedComponentsVector; @@ -100,7 +100,7 @@ void setBankVisited(PanelsSurfaceCalculator &self, const object &componentInfo, } } -size_t findNumDetectors(PanelsSurfaceCalculator &self, const object &componentInfo, const list &components) { +size_t findNumDetectors(const PanelsSurfaceCalculator &self, const object &componentInfo, const list &components) { const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); std::vector componentsVector; for (size_t i = 0; i < len(components); i++) { @@ -109,7 +109,7 @@ size_t findNumDetectors(PanelsSurfaceCalculator &self, const object &componentIn return self.findNumDetectors(*(cInfoSharedPtr.get()), componentsVector); } -list calcBankRotation(PanelsSurfaceCalculator &self, const list &detPos, list normal, const list &zAxis, +list calcBankRotation(const PanelsSurfaceCalculator &self, const list &detPos, list normal, const list &zAxis, const list &yAxis, const list &samplePosition) { const auto bankRotation = self.calcBankRotation(pythonListToV3D(detPos), pythonListToV3D(normal), pythonListToV3D(zAxis), @@ -123,8 +123,9 @@ list calcBankRotation(PanelsSurfaceCalculator &self, const list &detPos, list no return quat; } -list transformedBoundingBoxPoints(PanelsSurfaceCalculator &self, const object &componentInfo, size_t detectorIndex, - const list &refPos, const list &rotation, const list &xaxis, const list &yaxis) { +list transformedBoundingBoxPoints(const PanelsSurfaceCalculator &self, const object &componentInfo, + size_t detectorIndex, const list &refPos, const list &rotation, const list &xaxis, + const list &yaxis) { const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); Mantid::Kernel::Quat quatRotation{extract(rotation[0]), extract(rotation[1]), extract(rotation[2]), extract(rotation[3])}; @@ -162,7 +163,7 @@ list getAllTubeDetectorFlatGroupParents(PanelsSurfaceCalculator &self, const obj return pyAllTubeGroupParents; } -tuple getSideBySideViewPos(PanelsSurfaceCalculator &self, const object &componentInfo, const object &instrument, +tuple getSideBySideViewPos(const PanelsSurfaceCalculator &self, const object &componentInfo, const object &instrument, const size_t componentIndex) { const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); const std::shared_ptr instrumentSharedPtr = extract>(instrument); diff --git a/buildconfig/CMake/CppCheck_Suppressions.txt.in b/buildconfig/CMake/CppCheck_Suppressions.txt.in index 137280520244..1afc1380cea6 100644 --- a/buildconfig/CMake/CppCheck_Suppressions.txt.in +++ b/buildconfig/CMake/CppCheck_Suppressions.txt.in @@ -288,7 +288,6 @@ useInitializationList:${CMAKE_SOURCE_DIR}/Framework/DataObjects/src/TimeSplitter returnByReference:${CMAKE_SOURCE_DIR}/Framework/Geometry/inc/MantidGeometry/Crystal/BraggScattererInCrystalStructure.h:36 returnByReference:${CMAKE_SOURCE_DIR}/Framework/Geometry/inc/MantidGeometry/Crystal/CrystalStructure.h:76 virtualCallInConstructor:${CMAKE_SOURCE_DIR}/Framework/Geometry/inc/MantidGeometry/Crystal/UnitCell.h:212 -uselessOverride:${CMAKE_SOURCE_DIR}/Framework/Geometry/inc/MantidGeometry/Instrument.h:217 useStlAlgorithm:${CMAKE_SOURCE_DIR}/Framework/Geometry/inc/MantidGeometry/MDGeometry/MDImplicitFunction.h:96 useStlAlgorithm:${CMAKE_SOURCE_DIR}/Framework/Geometry/inc/MantidGeometry/MDGeometry/MDImplicitFunction.h:112 useStlAlgorithm:${CMAKE_SOURCE_DIR}/Framework/Geometry/inc/MantidGeometry/MDGeometry/MDImplicitFunction.h:128 From dea2ccb786db5fea8dd5624952baa1c66d51a3df Mon Sep 17 00:00:00 2001 From: James Clarke Date: Thu, 18 Sep 2025 14:07:17 +0100 Subject: [PATCH 17/23] Add comments to PanelsSurfaceCalculator --- Framework/API/src/PanelsSurfaceCalculator.cpp | 83 ++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/Framework/API/src/PanelsSurfaceCalculator.cpp b/Framework/API/src/PanelsSurfaceCalculator.cpp index df025f37cc6a..b8266ce23b1c 100644 --- a/Framework/API/src/PanelsSurfaceCalculator.cpp +++ b/Framework/API/src/PanelsSurfaceCalculator.cpp @@ -19,6 +19,7 @@ using Mantid::Kernel::V3D; namespace Mantid::API { /** * Given the z axis, define the x and y ones. + * * @param zaxis :: A given vector in 3d space to become the z axis of a * coordinate system. * @param xaxis :: An output arbitrary vector perpendicular to zaxis. @@ -41,6 +42,13 @@ void PanelsSurfaceCalculator::setupBasisAxes(const Mantid::Kernel::V3D &zaxis, M xaxis = yaxis.cross_prod(zaxis); } +/** + * Returns the four corners of the specified panel + * + * @param componentInfo :: ComponentInfo object from the workspace + * @param rootIndex :: Index of panel + * @returns :: Panel Corners + */ std::vector PanelsSurfaceCalculator::retrievePanelCorners(const Mantid::Geometry::ComponentInfo &componentInfo, const size_t rootIndex) const { @@ -97,6 +105,12 @@ PanelsSurfaceCalculator::retrievePanelCorners(const Mantid::Geometry::ComponentI return corners; } +/** + * Calculate the normal vector to a panel + * + * @param panelCorners :: The four panel corner locations + * @returns :: Normal vector + */ Mantid::Kernel::V3D PanelsSurfaceCalculator::calculatePanelNormal(const std::vector &panelCorners) const { // find the normal @@ -106,6 +120,15 @@ PanelsSurfaceCalculator::calculatePanelNormal(const std::vector &tubes, const Mantid::Kernel::V3D &normal) { for (auto tube : tubes) { @@ -119,6 +142,13 @@ bool PanelsSurfaceCalculator::isBankFlat(const ComponentInfo &componentInfo, siz return true; } +/** + * Calculate the normal vector of a bank of detectors + * + * @param componentInfo :: ComponentInfo object from the workspace + * @param tubes :: Component indices of the tubes in the bank + * @returns Bank normal vector + */ Mantid::Kernel::V3D PanelsSurfaceCalculator::calculateBankNormal(const ComponentInfo &componentInfo, const std::vector &tubes) { // calculate normal from first two tubes in bank as before @@ -146,7 +176,13 @@ Mantid::Kernel::V3D PanelsSurfaceCalculator::calculateBankNormal(const Component return normal; } -// Recursively set all detectors and subcomponents of a bank as visited +/** + * Recursively set all detectors and subcomponents of a bank as visited + * + * @param componentInfo :: ComponentInfo object from the workspace + * @param bankIndex :: Component index of the bank + * @param visitedComponents :: Vector keeping track of which components have been visited + */ void PanelsSurfaceCalculator::setBankVisited(const ComponentInfo &componentInfo, size_t bankIndex, std::vector &visitedComponents) const { const auto &children = componentInfo.children(bankIndex); @@ -160,6 +196,13 @@ void PanelsSurfaceCalculator::setBankVisited(const ComponentInfo &componentInfo, } } +/** + * How many detectors are there in the given list of component indices? + * + * @param componentInfo :: ComponentInfo object from the workspace + * @param components :: Component indices to check + * @return :: Number of detectors + */ size_t PanelsSurfaceCalculator::findNumDetectors(const ComponentInfo &componentInfo, const std::vector &components) const { return std::accumulate( @@ -173,6 +216,10 @@ size_t PanelsSurfaceCalculator::findNumDetectors(const ComponentInfo &componentI * * @param detPos :: Position of a detector of the bank. * @param normal :: Normal to the bank's plane. + * @param zAxis :: Direction to the viewer + * @param yAxis :: Perpendicular axis + * @param samplePosition :: Sample position + * @returns :: Rotation */ Mantid::Kernel::Quat PanelsSurfaceCalculator::calcBankRotation(const V3D &detPos, V3D normal, const V3D &zAxis, const V3D &yAxis, const V3D &samplePosition) const { @@ -213,6 +260,17 @@ Mantid::Kernel::Quat PanelsSurfaceCalculator::calcBankRotation(const V3D &detPos return requiredRotation; } +/** + * Transforms bounding box of a detector + * + * @param componentInfo :: ComponentInfo object from the workspace + * @param detectorIndex :: Component index of the detector + * @param refPos :: Reference position + * @param rotation :: Rotation to apply + * @param xaxis :: X axis + * @param yaxis :: Y axis + * @returns Transformed bounding box points + */ std::vector PanelsSurfaceCalculator::transformedBoundingBoxPoints(const ComponentInfo &componentInfo, size_t detectorIndex, const V3D &refPos, const Mantid::Kernel::Quat &rotation, @@ -229,6 +287,13 @@ PanelsSurfaceCalculator::transformedBoundingBoxPoints(const ComponentInfo &compo return {bb0, bb1}; } +/** + * Perform a specified operation on all the components + * + * @param componentInfo :: ComponentInfo object from the workspace + * @param operation :: Operation to perform on each component + * @returns Detector IDs for each component + */ std::vector> PanelsSurfaceCalculator::examineAllComponents( const ComponentInfo &componentInfo, std::function(const ComponentInfo &, size_t, std::vector &)> operation) { @@ -248,6 +313,14 @@ std::vector> PanelsSurfaceCalculator::examineAllComponents( return detectorIDs; } +/** + * Parent indices of tubes + * + * @param componentInfo :: ComponentInfo object from the workspace + * @param rootIndex :: Component index to check + * @param visited :: Vector tracking which components have been checked + * @returns Parent IDs + */ std::vector PanelsSurfaceCalculator::tubeDetectorParentIDs(const ComponentInfo &componentInfo, size_t rootIndex, std::vector &visited) { const auto componentType = componentInfo.componentType(rootIndex); @@ -307,6 +380,14 @@ std::vector PanelsSurfaceCalculator::tubeDetectorParentIDs(const Compone return tubes; } +/** + * Gives the specified side-by-side view position from the IDF + * + * @param componentInfo :: ComponentInfo object from the workspace + * @param instrument :: Instrument object from the workspace + * @param componentIndex :: Component index to check + * @returns :: Side-by-side position from the IDF + */ std::optional PanelsSurfaceCalculator::getSideBySideViewPos(const ComponentInfo &componentInfo, const Instrument_const_sptr instrument, const size_t componentIndex) const { From a82ae86ee6fef5cf9c4b322f109e264a9edb5be4 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Mon, 22 Sep 2025 14:48:00 +0100 Subject: [PATCH 18/23] Handle case of bank arrays of differing sizes To flatten the detector IDs of the tube banks and the grid banks together, we need to use np.concatenate, because we can't make an array with 2 lists of different lengths. --- .../instrumentview/instrumentview/Projections/SideBySide.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py index f92035feb258..cdae36dd524a 100644 --- a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py @@ -102,7 +102,7 @@ def _construct_flat_panels(self, workspace: Workspace2D) -> None: grids = self._construct_rectangles_and_grids(workspace) tubes = self._construct_tube_banks(workspace.componentInfo()) self._flat_banks = grids + tubes - detectors_in_banks = np.array([d.detector_ids for d in self._flat_banks]).flatten() + detectors_in_banks = np.empty(0) if len(self._flat_banks) == 0 else np.concatenate([d.detector_ids for d in self._flat_banks]) remaining_detectors_bank = self._create_flat_bank_with_missing_detectors(detectors_in_banks) if remaining_detectors_bank: self._flat_banks.append(remaining_detectors_bank) From f989df5d419ea60f325951df2cda4c8bb8e8e75f Mon Sep 17 00:00:00 2001 From: James Clarke Date: Tue, 23 Sep 2025 12:27:53 +0100 Subject: [PATCH 19/23] Add tests for SideBySide class --- .../Projections/test/test_SideBySide.py | 204 +++++++++++++++++- 1 file changed, 201 insertions(+), 3 deletions(-) diff --git a/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py index 883f2fed18bf..98c42bc732d9 100644 --- a/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py @@ -4,9 +4,12 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + +from instrumentview.Projections.SideBySide import SideBySide, FlatBankInfo + import numpy as np -import unittest.mock +from unittest.mock import MagicMock, patch import unittest +from mantid.kernel import Quat class TestSideBySideProjection(unittest.TestCase): @@ -16,5 +19,200 @@ def setUpClass(cls): cls.sample_position = np.array([0, 0, 0]) cls.detector_positions = np.array([[0, 1, 0], [2, 1, 0], [-2, 1, 0]]) - def test_calculate_2d_coordinates(self): - self.assertTrue(False) + def _create_flat_bank_info(self, use_relative_positions: bool, detector_ids: list[int] = [5, 10, 15]) -> FlatBankInfo: + flat_bank = FlatBankInfo() + flat_bank.reference_position = np.zeros(3) + flat_bank.detector_ids = detector_ids + if use_relative_positions: + flat_bank.relative_projected_positions = self.detector_positions.copy() + else: + flat_bank.steps = [1, 1, 1] + flat_bank.pixels = [3, 1, 0] + + return flat_bank + + def test_flat_bank_info_translate_relative(self): + flat_bank = self._create_flat_bank_info(use_relative_positions=True) + flat_bank._transform_positions_so_origin_bottom_left = MagicMock() + flat_bank.calculate_projected_positions() + flat_bank.translate([1, 1, 1]) + np.testing.assert_allclose([1, 1, 1], flat_bank.reference_position) + # We should have translated all points by [1,1,1] + np.testing.assert_allclose([[1, 2, 1], [3, 2, 1], [-1, 2, 1]], list(flat_bank.detector_id_position_map.values())) + flat_bank._transform_positions_so_origin_bottom_left.assert_called_once() + + def test_flat_bank_info_translate_pixels(self): + flat_bank = self._create_flat_bank_info(use_relative_positions=False) + flat_bank._transform_positions_so_origin_bottom_left = MagicMock() + flat_bank.calculate_projected_positions() + flat_bank.translate([-5, 10, 0]) + np.testing.assert_allclose([-5, 10, 0], flat_bank.reference_position) + # Initial positions are from pixels and steps, then translated + np.testing.assert_allclose([[-5, 10, 0], [-4, 10, 0], [-3, 10, 0]], list(flat_bank.detector_id_position_map.values())) + flat_bank._transform_positions_so_origin_bottom_left.assert_not_called() + + def test_flat_bank_transform_positions_to_bottom_left(self): + flat_bank = self._create_flat_bank_info(use_relative_positions=True) + flat_bank._transform_positions_so_origin_bottom_left() + np.testing.assert_allclose([[2, 0, 0], [4, 0, 0], [0, 0, 0]], list(flat_bank.relative_projected_positions)) + + def test_flat_bank_calculate_projected_positions_relative(self): + flat_bank = self._create_flat_bank_info(use_relative_positions=True) + flat_bank.calculate_projected_positions() + np.testing.assert_allclose([[2, 0, 0], [4, 0, 0], [0, 0, 0]], list(flat_bank.detector_id_position_map.values())) + + def test_flat_bank_calculate_projected_positions_pixels(self): + flat_bank = self._create_flat_bank_info(use_relative_positions=False) + flat_bank.calculate_projected_positions() + np.testing.assert_allclose([[0, 0, 0], [1, 0, 0], [2, 0, 0]], list(flat_bank.detector_id_position_map.values())) + + def _create_side_by_side(self, detector_ids: list[int], has_other_detectors: bool) -> SideBySide: + mock_workspace = MagicMock() + mock_detector_info = MagicMock() + mock_detector_info.detectorIDs.return_value = detector_ids + [100] if has_other_detectors else detector_ids + mock_workspace.detectorInfo.return_value = mock_detector_info + side_by_side = SideBySide( + workspace=mock_workspace, + detector_ids=detector_ids, + sample_position=np.zeros(3), + root_position=np.zeros(3), + detector_positions=np.array([[1, 1, 1], [0, 1, 0], [0, 0, 1]]), + axis=np.array([0, 0, 1]), + ) + return side_by_side + + @patch("instrumentview.Projections.SideBySide.PanelsSurfaceCalculator") + def test_create_side_by_side_no_extra_detectors(self, mock_panels_surface_calculator): + side_by_side = self._create_side_by_side([5, 10, 15], False) + side_by_side._workspace.detectorInfo().detectorIDs.assert_called_once() + self.assertEqual(list(range(3)), list(side_by_side._component_index_detector_id_map.keys())) + self.assertEqual([5, 10, 15], list(side_by_side._component_index_detector_id_map.values())) + mock_panels_surface_calculator.assert_called_once() + + @patch("instrumentview.Projections.SideBySide.PanelsSurfaceCalculator") + def test_create_side_by_side_subset_detectors(self, mock_panels_surface_calculator): + side_by_side = self._create_side_by_side([5, 10, 15], True) + side_by_side._workspace.detectorInfo().detectorIDs.assert_called_once() + self.assertEqual(list(range(3)), list(side_by_side._component_index_detector_id_map.keys())) + self.assertEqual([5, 10, 15], list(side_by_side._component_index_detector_id_map.values())) + mock_panels_surface_calculator.assert_called_once() + + @patch("instrumentview.Projections.SideBySide.PanelsSurfaceCalculator") + def test_calculate_axes(self, mock_panels_surface_calculator): + side_by_side = self._create_side_by_side(list(range(3)), False) + side_by_side._calculator.setupBasisAxes.assert_called_once_with([0, 0, 0], [0, 0, 0], [0, 0, 1]) + + @patch("instrumentview.Projections.SideBySide.SideBySide._arrange_panels") + @patch("instrumentview.Projections.Projection.Projection._calculate_detector_coordinates") + @patch("instrumentview.Projections.SideBySide.SideBySide._create_flat_bank_with_missing_detectors") + @patch("instrumentview.Projections.SideBySide.SideBySide._construct_tube_banks") + @patch("instrumentview.Projections.SideBySide.SideBySide._construct_rectangles_and_grids") + @patch("instrumentview.Projections.SideBySide.PanelsSurfaceCalculator") + def test_construct_flat_panels( + self, + mock_panels_surface_calculator, + mock_construct_rectangles, + mock_construct_tube_banks, + mock_create_flat_bank, + mock_calculate_coordinates, + mock_arrange_panels, + ): + side_by_side = self._create_side_by_side([5, 10, 15, 20, 25], True) + mock_construct_rectangles.return_value = [self._create_flat_bank_info(True, detector_ids=[5, 10, 15])] + mock_construct_tube_banks.return_value = [self._create_flat_bank_info(True, detector_ids=[20, 25])] + mock_create_flat_bank.return_value = [] + side_by_side._construct_flat_panels(side_by_side._workspace) + detector_map = side_by_side._detector_id_to_flat_bank_map + self.assertEqual([5, 10, 15, 20, 25], list(detector_map.keys())) + mock_arrange_panels.assert_called_once() + self.assertEqual(2, len(side_by_side._flat_banks)) + mock_construct_rectangles.assert_called_once() + mock_construct_tube_banks.assert_called_once() + + @patch("instrumentview.Projections.SideBySide.PanelsSurfaceCalculator") + def test_construct_rectangles_and_grids(self, mock_panels_surface_calculator): + side_by_side = self._create_side_by_side([5, 10, 15, 20, 25], True) + ws = side_by_side._workspace + mock_bank = MagicMock() + mock_bank.getPos.return_value = [2, 2, 2] + mock_bank.getRotation.return_value = Quat(1, 1, 0, 0) + mock_bank.minDetectorID.return_value = 5 + mock_bank.maxDetectorID.return_value = 5 + ws.getInstrument().findGridDetectors.return_value = [mock_bank] + flat_banks = side_by_side._construct_rectangles_and_grids(side_by_side._workspace) + self.assertEqual(1, len(flat_banks)) + self.assertEqual([5], flat_banks[0].detector_ids) + side_by_side._calculator.getSideBySideViewPos.assert_called_once() + + @patch("instrumentview.Projections.Projection.Projection._calculate_detector_coordinates") + @patch("instrumentview.Projections.SideBySide.PanelsSurfaceCalculator") + def test_construct_tube_banks(self, mock_panels_surface_calculator, mock_calculate_detector_coords): + detector_IDs = [5, 10, 15, 20, 25] + side_by_side = self._create_side_by_side(detector_IDs, True) + side_by_side._component_index_detector_id_map = {id: id for id in detector_IDs} + mock_component_info = side_by_side._workspace.componentInfo() + side_by_side._calculator.getAllTubeDetectorFlatGroupParents.return_value = [[3]] + side_by_side._calculator.calculateBankNormal.return_value = [0, 0, -1] + side_by_side._calculator.calcBankRotation.return_value = Quat(1, 0, 1, 0) + mock_component_info.children.return_value = detector_IDs + mock_component_info.position.return_value = [0, 1, 0] + tube_banks = side_by_side._construct_tube_banks(mock_component_info) + self.assertEqual(1, len(tube_banks)) + self.assertEqual(detector_IDs, tube_banks[0].detector_ids) + side_by_side._calculator.getAllTubeDetectorFlatGroupParents.assert_called_once_with(mock_component_info) + side_by_side._calculator.calculateBankNormal.assert_called_once() + side_by_side._calculator.calcBankRotation.assert_called_once() + + @patch("instrumentview.Projections.SideBySide.PanelsSurfaceCalculator") + def test_calculate_projected_positions_plane(self, mock_panels_surface_calculator): + # Put three points in a plane, should get the same points back + side_by_side = self._create_side_by_side([5, 10, 15], False) + points = np.array([[0, 1, 0], [1, 0, 0], [1, 1, 0]]) + normal = np.array([0, 0, -1]) + flat_bank = self._create_flat_bank_info(True) + flat_bank.reference_position = np.zeros(3) + projected_points = side_by_side._calculate_projected_positions(points, normal, flat_bank) + self.assertEqual(3, len(projected_points)) + np.testing.assert_allclose(points, projected_points) + + @patch("instrumentview.Projections.SideBySide.PanelsSurfaceCalculator") + def test_calculate_projected_positions_angle(self, mock_panels_surface_calculator): + # Points above/below plane should be projected correctly + side_by_side = self._create_side_by_side([5, 10, 15], False) + points = np.array([[0, 1, 0], [1, 0, 0.5], [1, 1, -0.5]]) + normal = np.array([0, 0, -1]) + flat_bank = self._create_flat_bank_info(True) + flat_bank.reference_position = np.zeros(3) + projected_points = side_by_side._calculate_projected_positions(points, normal, flat_bank) + self.assertEqual(3, len(projected_points)) + np.testing.assert_allclose([[0, 1, 0], [1, 0, 0], [1, 1, 0]], projected_points) + + @patch("instrumentview.Projections.Projection.Projection._calculate_detector_coordinates") + @patch("instrumentview.Projections.SideBySide.PanelsSurfaceCalculator") + def test_create_flat_bank_with_missing_detectors(self, mock_panels_surface_calculator, mock_calculate_detector_coords): + side_by_side = self._create_side_by_side([5, 10, 15, 20, 25], False) + flat_bank = side_by_side._create_flat_bank_with_missing_detectors(np.array([5])) + self.assertTrue(type(flat_bank) is FlatBankInfo) + self.assertEqual([10, 15, 20, 25], flat_bank.detector_ids) + separation = 0.01 + # Four remaining detectors should be arranged in a square with the default separation between them + np.testing.assert_allclose( + [[0, 0, 0], [0, separation, 0], [separation, 0, 0], [separation, separation, 0]], flat_bank.relative_projected_positions + ) + + @patch("instrumentview.Projections.Projection.Projection._calculate_detector_coordinates") + @patch("instrumentview.Projections.SideBySide.PanelsSurfaceCalculator") + def test_arrange_panels(self, mock_panels_surface_calculator, mock_calculate_detector_coords): + detector_ids = [5, 10, 15, 20, 25] + side_by_side = self._create_side_by_side(detector_ids, False) + mock_flat_bank = MagicMock() + mock_flat_bank.dimensions = [1, 1, 1] + mock_flat_bank.has_position_in_idf = False + mock_flat_bank.reference_position = [5, 5, 5] + side_by_side._flat_banks = [mock_flat_bank] + side_by_side._arrange_panels() + mock_flat_bank.translate.assert_called_once() + + +if __name__ == "__main__": + unittest.main() From 403e01cbfdf069be8ec990949b6707480c1881d4 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Tue, 23 Sep 2025 16:20:17 +0100 Subject: [PATCH 20/23] Fixes following CodeRabbit review --- .../inc/MantidAPI/PanelsSurfaceCalculator.h | 1 + Framework/API/src/PanelsSurfaceCalculator.cpp | 9 +++-- .../src/Exports/PanelsSurfaceCalculator.cpp | 2 +- .../mantid/api/PanelsSurfaceCalculatorTest.py | 9 +++-- .../instrumentview/Projections/SideBySide.py | 38 +++++++++---------- 5 files changed, 31 insertions(+), 28 deletions(-) diff --git a/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h index b37750a8fce0..b8779df2d79f 100644 --- a/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h +++ b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h @@ -16,6 +16,7 @@ #include #include +#include using Mantid::Geometry::ComponentInfo; using Mantid::Geometry::Instrument; diff --git a/Framework/API/src/PanelsSurfaceCalculator.cpp b/Framework/API/src/PanelsSurfaceCalculator.cpp index b8266ce23b1c..83433e4736f5 100644 --- a/Framework/API/src/PanelsSurfaceCalculator.cpp +++ b/Framework/API/src/PanelsSurfaceCalculator.cpp @@ -12,6 +12,9 @@ #include "MantidGeometry/Rendering/GeometryHandler.h" #include "MantidGeometry/Rendering/ShapeInfo.h" +#include +#include + using namespace Mantid::Geometry; using Mantid::Beamline::ComponentType; using Mantid::Kernel::V3D; @@ -134,7 +137,7 @@ bool PanelsSurfaceCalculator::isBankFlat(const ComponentInfo &componentInfo, siz for (auto tube : tubes) { const auto &children = componentInfo.children(tube); const auto vector = normalize(componentInfo.position(children[0]) - componentInfo.position(children[1])); - if (fabs(vector.scalar_prod(normal)) > Mantid::Kernel::Tolerance) { + if (std::abs(vector.scalar_prod(normal)) > Mantid::Kernel::Tolerance) { this->g_log.warning() << "Assembly " << componentInfo.name(bankIndex) << " isn't flat.\n"; return false; } @@ -348,10 +351,10 @@ std::vector PanelsSurfaceCalculator::tubeDetectorParentIDs(const Compone // Try grandparent of the tube supplied tube initially if (componentInfo.hasParent(bankIndex0)) { bankIndex = componentInfo.parent(bankIndex0); - auto bankChildren = &componentInfo.children(bankIndex); + const auto &bankChildren = componentInfo.children(bankIndex); // Go down the tree to find all the tubes. - for (auto index : *bankChildren) + for (const auto index : bankChildren) addTubes(index, tubes); if (tubes.empty()) { this->setBankVisited(componentInfo, bankIndex, visited); diff --git a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp index c1021fa15066..b09e22cd4f44 100644 --- a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp +++ b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp @@ -214,7 +214,7 @@ void export_PanelsSurfaceCalculator() { (arg("self"), arg("componentInfo"), arg("detectorIndex"), arg("refPos"), arg("rotation"), arg("xaxis"), arg("yaxis")), "Transforms a component's bounding box based on reference position and rotation. The rotation should be " - "provided as a list containing the real and imaginary parts of a quarternion (length 4).") + "provided as a list containing the real and imaginary parts of a quarternion (w, i, j, k).") .def("getAllTubeDetectorFlatGroupParents", &getAllTubeDetectorFlatGroupParents, (arg("self"), arg("componentInfo")), "Returns the parent component indices of detectors of all groups of tubes arranged in flat banks") diff --git a/Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py b/Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py index 67dc94205d97..b39ff58bdc27 100644 --- a/Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py +++ b/Framework/PythonInterface/test/python/mantid/api/PanelsSurfaceCalculatorTest.py @@ -9,6 +9,7 @@ from mantid.api import PanelsSurfaceCalculator from mantid.simpleapi import CreateSampleWorkspace import numpy as np +from mantid.kernel import Quat, V3D class PanelsSurfaceCalculatorTest(unittest.TestCase): @@ -74,15 +75,15 @@ def test_calcBankRotation(self): np.testing.assert_allclose(expected_rotation, rotation, atol=1e-9) def test_transformedBoundingBoxPoints(self): - rotation = [45, 1, 0, 0] + quat = Quat(45, V3D(1, 0, 0)) bounding_box_points = self._calculator.transformedBoundingBoxPoints( - self._ws.componentInfo(), 25, [0, 0, 0], rotation, [1, 0, 0], [0, 1, 0] + self._ws.componentInfo(), 25, [0, 0, 0], [quat.real(), quat.imagI(), quat.imagJ(), quat.imagK()], [1, 0, 0], [0, 1, 0] ) point_a = bounding_box_points[0] point_b = bounding_box_points[1] tol = 1e-3 - np.testing.assert_allclose([-0.004, -0.222], point_a, atol=tol) - np.testing.assert_allclose([0.004, -0.190], point_b, atol=tol) + np.testing.assert_allclose([-0.004, -3.533], point_a, atol=tol) + np.testing.assert_allclose([0.004, -3.516], point_b, atol=tol) if __name__ == "__main__": diff --git a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py index cdae36dd524a..7866337d36af 100644 --- a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py @@ -11,19 +11,22 @@ from mantid.dataobjects import Workspace2D from mantid.geometry import ComponentInfo from scipy.spatial.transform import Rotation +from dataclasses import dataclass, field +from typing import List, Dict, Optional +@dataclass class FlatBankInfo: """Information about a flat bank detector.""" - rotation: Rotation - detector_ids: list[int] - reference_position: np.ndarray - detector_id_position_map: dict[int, np.ndarray] = {} - dimensions: np.ndarray - steps: list[int] - pixels: list[int] - relative_projected_positions: np.ndarray = np.array([]) + rotation: Optional[Rotation] = None + detector_ids: List[int] = field(default_factory=list) + reference_position: np.ndarray = field(default_factory=lambda: np.zeros(3)) + detector_id_position_map: Dict[int, np.ndarray] = field(default_factory=dict) + dimensions: np.ndarray = field(default_factory=lambda: np.zeros(3)) + steps: List[float] = field(default_factory=list) + pixels: List[int] = field(default_factory=list) + relative_projected_positions: np.ndarray = field(default_factory=lambda: np.empty((0, 3))) has_position_in_idf: bool = False def translate(self, shift: np.ndarray): @@ -58,13 +61,6 @@ def calculate_projected_positions(self) -> None: class SideBySide(Projection): - _detector_id_to_flat_bank_map: dict[int, FlatBankInfo] = {} - _flat_banks: list[FlatBankInfo] = [] - _calculator: PanelsSurfaceCalculator - _workspace: Workspace2D - _component_index_detector_id_map: dict[int, int] - _detector_id_component_index_map: dict[int, int] - def __init__( self, workspace: Workspace2D, @@ -75,6 +71,8 @@ def __init__( axis: np.ndarray, ): self._calculator = PanelsSurfaceCalculator() + self._detector_id_to_flat_bank_map: dict[int, FlatBankInfo] = {} + self._flat_banks: list[FlatBankInfo] = [] self._detector_ids = detector_ids self._workspace = workspace all_detector_ids = np.array(workspace.detectorInfo().detectorIDs()) @@ -98,7 +96,6 @@ def _calculate_axes(self, root_position: np.ndarray) -> None: self._y_axis = y def _construct_flat_panels(self, workspace: Workspace2D) -> None: - self._detector_id_to_flat_bank_map.clear() grids = self._construct_rectangles_and_grids(workspace) tubes = self._construct_tube_banks(workspace.componentInfo()) self._flat_banks = grids + tubes @@ -131,7 +128,7 @@ def _construct_rectangles_and_grids(self, workspace: Workspace2D) -> list[FlatBa flat_bank.detector_id_position_map.clear() flat_bank.reference_position = np.array(bank.getPos()) rotation = bank.getRotation() - flat_bank.rotation = Rotation.from_quat([rotation.real(), rotation.imagI(), rotation.imagJ(), rotation.imagK()]) + flat_bank.rotation = Rotation.from_quat([rotation.imagI(), rotation.imagJ(), rotation.imagK(), rotation.real()]) flat_bank.detector_ids = list(range(bank.minDetectorID(), bank.maxDetectorID() + 1)) flat_bank.dimensions = np.abs([bank.xsize(), bank.ysize(), bank.zsize()]) flat_bank.steps = np.abs([bank.xstep(), bank.ystep(), bank.zstep()]) @@ -168,7 +165,8 @@ def _construct_tube_banks(self, component_info: ComponentInfo) -> list[FlatBankI list(self._y_axis), list(self._sample_position), ) - flat_bank.rotation = Rotation.from_quat([rotation[0], rotation[1], rotation[2], rotation[3]]) + # SciPy from_quat expects (x, y, z, w) + flat_bank.rotation = Rotation.from_quat([rotation[1], rotation[2], rotation[3], rotation[0]]) flat_bank.detector_ids = detectors flat_bank.relative_projected_positions = self._calculate_projected_positions( np.array([component_info.position(int(d)) for d in detector_component_indices]), normal, flat_bank @@ -189,7 +187,7 @@ def _calculate_projected_positions(self, detector_positions: np.ndarray, normal: x_axis = np.array([1, 0, 0]) if np.allclose(x_axis, normal): # if v and normal are parallel x_axis = np.array([0, 1, 0]) - u_plane = x_axis + (x_axis.dot(normal)) * normal + u_plane = x_axis - (x_axis.dot(normal)) * normal u_plane = u_plane / np.linalg.norm(u_plane) v_plane = np.cross(u_plane, normal) return np.array([[np.dot(p, u_plane), np.dot(p, v_plane), 0] for p in positions]) @@ -239,7 +237,7 @@ def _create_flat_bank_with_missing_detectors(self, detectors_already_in_banks: n def _arrange_panels(self) -> None: max_dims = np.max([b.dimensions for b in self._flat_banks], axis=0) number_banks = len(self._flat_banks) - banks_per_row = np.ceil(np.sqrt(number_banks)) + banks_per_row = int(np.ceil(np.sqrt(number_banks))) position = np.zeros(3) banks_arranged = 0 space_factor = 1.2 From 750eb5630b16a681ae7841179165c3a957134d3c Mon Sep 17 00:00:00 2001 From: James Clarke Date: Tue, 23 Sep 2025 16:48:54 +0100 Subject: [PATCH 21/23] Further improvements from code review --- .../inc/MantidAPI/PanelsSurfaceCalculator.h | 2 +- Framework/API/src/PanelsSurfaceCalculator.cpp | 2 +- .../Geometry/inc/MantidGeometry/Instrument.h | 1 + .../src/Exports/PanelsSurfaceCalculator.cpp | 4 +--- .../instrumentview/FullInstrumentViewModel.py | 19 ++++++++++--------- .../Projections/CylindricalProjection.py | 3 ++- .../instrumentview/Projections/Projection.py | 8 ++++---- .../instrumentview/Projections/SideBySide.py | 10 +++++++--- .../Projections/SphericalProjection.py | 3 ++- .../Projections/test/test_SideBySide.py | 4 ++-- .../instrumentview/test/test_model.py | 4 ++++ 11 files changed, 35 insertions(+), 25 deletions(-) diff --git a/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h index b8779df2d79f..f984381d3005 100644 --- a/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h +++ b/Framework/API/inc/MantidAPI/PanelsSurfaceCalculator.h @@ -47,7 +47,7 @@ class MANTID_API_DLL PanelsSurfaceCalculator { const ComponentInfo &componentInfo, std::function(const ComponentInfo &, size_t, std::vector &)> operation); std::optional getSideBySideViewPos(const ComponentInfo &componentInfo, - const Instrument_const_sptr instrument, + const Instrument_const_sptr &instrument, const size_t componentIndex) const; private: diff --git a/Framework/API/src/PanelsSurfaceCalculator.cpp b/Framework/API/src/PanelsSurfaceCalculator.cpp index 83433e4736f5..496b4b8866a8 100644 --- a/Framework/API/src/PanelsSurfaceCalculator.cpp +++ b/Framework/API/src/PanelsSurfaceCalculator.cpp @@ -392,7 +392,7 @@ std::vector PanelsSurfaceCalculator::tubeDetectorParentIDs(const Compone * @returns :: Side-by-side position from the IDF */ std::optional PanelsSurfaceCalculator::getSideBySideViewPos(const ComponentInfo &componentInfo, - const Instrument_const_sptr instrument, + const Instrument_const_sptr &instrument, const size_t componentIndex) const { const auto *componentID = componentInfo.componentID(componentIndex); const auto component = instrument->getComponentByID(componentID); diff --git a/Framework/Geometry/inc/MantidGeometry/Instrument.h b/Framework/Geometry/inc/MantidGeometry/Instrument.h index 7214ff9806b9..77c6cab7cc1d 100644 --- a/Framework/Geometry/inc/MantidGeometry/Instrument.h +++ b/Framework/Geometry/inc/MantidGeometry/Instrument.h @@ -9,6 +9,7 @@ #include "MantidGeometry/DllConfig.h" #include "MantidGeometry/IDetector.h" #include "MantidGeometry/Instrument/CompAssembly.h" +#include "MantidGeometry/Instrument/GridDetector.h" #include "MantidGeometry/Instrument/ObjComponent.h" #include "MantidGeometry/Instrument/RectangularDetector.h" #include "MantidGeometry/Instrument_fwd.h" diff --git a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp index b09e22cd4f44..9f650ff7b94e 100644 --- a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp +++ b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp @@ -4,8 +4,6 @@ // NScD Oak Ridge National Laboratory, European Spallation Source, // Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS // SPDX - License - Identifier: GPL - 3.0 + -#pragma once - #include "MantidAPI/PanelsSurfaceCalculator.h" #include "MantidAPI/MatrixWorkspace.h" #include @@ -214,7 +212,7 @@ void export_PanelsSurfaceCalculator() { (arg("self"), arg("componentInfo"), arg("detectorIndex"), arg("refPos"), arg("rotation"), arg("xaxis"), arg("yaxis")), "Transforms a component's bounding box based on reference position and rotation. The rotation should be " - "provided as a list containing the real and imaginary parts of a quarternion (w, i, j, k).") + "provided as a list containing the real and imaginary parts of a quaternion (w, i, j, k).") .def("getAllTubeDetectorFlatGroupParents", &getAllTubeDetectorFlatGroupParents, (arg("self"), arg("componentInfo")), "Returns the parent component indices of detectors of all groups of tubes arranged in flat banks") diff --git a/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py b/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py index 9228ed61a638..f3ebab16f9e0 100644 --- a/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py +++ b/qt/python/instrumentview/instrumentview/FullInstrumentViewModel.py @@ -11,20 +11,21 @@ from mantid.dataobjects import Workspace2D from mantid.simpleapi import CreateDetectorTable, ExtractSpectra, ConvertUnits, AnalysisDataService, SumSpectra, Rebin import numpy as np +from typing import ClassVar class FullInstrumentViewModel: """Model for the Instrument View Window. Will calculate detector positions, indices, and integrated counts that give the colours""" - _FULL_3D = "3D" - _SPHERICAL_X = "Spherical X" - _SPHERICAL_Y = "Spherical Y" - _SPHERICAL_Z = "Spherical Z" - _CYLINDRICAL_X = "Cylindrical X" - _CYLINDRICAL_Y = "Cylindrical Y" - _CYLINDRICAL_Z = "Cylindrical Z" - _SIDE_BY_SIDE = "Side by Side" - _PROJECTION_OPTIONS = [ + _FULL_3D: ClassVar[str] = "3D" + _SPHERICAL_X: ClassVar[str] = "Spherical X" + _SPHERICAL_Y: ClassVar[str] = "Spherical Y" + _SPHERICAL_Z: ClassVar[str] = "Spherical Z" + _CYLINDRICAL_X: ClassVar[str] = "Cylindrical X" + _CYLINDRICAL_Y: ClassVar[str] = "Cylindrical Y" + _CYLINDRICAL_Z: ClassVar[str] = "Cylindrical Z" + _SIDE_BY_SIDE: ClassVar[str] = "Side by Side" + _PROJECTION_OPTIONS: ClassVar[list[str]] = [ _FULL_3D, _SPHERICAL_X, _SPHERICAL_Y, diff --git a/qt/python/instrumentview/instrumentview/Projections/CylindricalProjection.py b/qt/python/instrumentview/instrumentview/Projections/CylindricalProjection.py index 570a6498ed4c..e49de8d343bc 100644 --- a/qt/python/instrumentview/instrumentview/Projections/CylindricalProjection.py +++ b/qt/python/instrumentview/instrumentview/Projections/CylindricalProjection.py @@ -17,7 +17,8 @@ def _calculate_2d_coordinates(self) -> tuple[np.ndarray, np.ndarray]: x = detector_relative_positions.dot(self._x_axis) y = detector_relative_positions.dot(self._y_axis) - v_scale = 1.0 / np.sqrt(x * x + y * y + z * z) + norm = np.sqrt(x * x + y * y + z * z) + v_scale = np.divide(1.0, norm, out=np.zeros_like(norm), where=norm > 0) v = z * v_scale u = -np.atan2(y, x) diff --git a/qt/python/instrumentview/instrumentview/Projections/Projection.py b/qt/python/instrumentview/instrumentview/Projections/Projection.py index 1455f7d89e85..0aa4cc70895e 100644 --- a/qt/python/instrumentview/instrumentview/Projections/Projection.py +++ b/qt/python/instrumentview/instrumentview/Projections/Projection.py @@ -21,10 +21,10 @@ def __init__( ): """For the given workspace and detectors, calculate 2D points with specified projection axis""" - self._sample_position = sample_position - self._root_position = root_position - self._detector_positions = np.array(detector_positions) - self._projection_axis = np.array(axis, dtype=np.float64) + self._sample_position = np.asarray(sample_position, dtype=np.float64) + self._root_position = np.asarray(root_position, dtype=np.float64) + self._detector_positions = np.asarray(detector_positions, dtype=np.float64) + self._projection_axis = np.asarray(axis, dtype=np.float64) self._x_axis = np.zeros_like(self._projection_axis, dtype=np.float64) self._y_axis = np.zeros_like(self._projection_axis, dtype=np.float64) diff --git a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py index 7866337d36af..13b26678031e 100644 --- a/qt/python/instrumentview/instrumentview/Projections/SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/SideBySide.py @@ -99,9 +99,13 @@ def _construct_flat_panels(self, workspace: Workspace2D) -> None: grids = self._construct_rectangles_and_grids(workspace) tubes = self._construct_tube_banks(workspace.componentInfo()) self._flat_banks = grids + tubes - detectors_in_banks = np.empty(0) if len(self._flat_banks) == 0 else np.concatenate([d.detector_ids for d in self._flat_banks]) + detectors_in_banks = ( + np.empty(0, dtype=int) + if len(self._flat_banks) == 0 + else np.concatenate([np.array(d.detector_ids, dtype=int) for d in self._flat_banks]) + ) remaining_detectors_bank = self._create_flat_bank_with_missing_detectors(detectors_in_banks) - if remaining_detectors_bank: + if remaining_detectors_bank is not None: self._flat_banks.append(remaining_detectors_bank) if len(self._flat_banks) == 0: @@ -192,7 +196,7 @@ def _calculate_projected_positions(self, detector_positions: np.ndarray, normal: v_plane = np.cross(u_plane, normal) return np.array([[np.dot(p, u_plane), np.dot(p, v_plane), 0] for p in positions]) - def _create_flat_bank_with_missing_detectors(self, detectors_already_in_banks: np.ndarray) -> FlatBankInfo: + def _create_flat_bank_with_missing_detectors(self, detectors_already_in_banks: np.ndarray) -> Optional[FlatBankInfo]: detectors = np.array(self._detector_ids) missing_detectors = np.setdiff1d(detectors, detectors_already_in_banks) number_of_detectors = len(missing_detectors) diff --git a/qt/python/instrumentview/instrumentview/Projections/SphericalProjection.py b/qt/python/instrumentview/instrumentview/Projections/SphericalProjection.py index 5b5ec7aed746..00f40cc3f166 100644 --- a/qt/python/instrumentview/instrumentview/Projections/SphericalProjection.py +++ b/qt/python/instrumentview/instrumentview/Projections/SphericalProjection.py @@ -20,5 +20,6 @@ def _calculate_2d_coordinates(self) -> tuple[np.ndarray, np.ndarray]: r = np.sqrt(x * x + y * y + v * v) u = -np.atan2(y, x) - v = -np.acos(v / r) + vr = np.divide(v, r, out=np.zeros_like(v), where=r > 0) + v = -np.acos(np.clip(vr, -1.0, 1.0)) return (u, v) diff --git a/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py index 98c42bc732d9..deb2387280f9 100644 --- a/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py @@ -19,10 +19,10 @@ def setUpClass(cls): cls.sample_position = np.array([0, 0, 0]) cls.detector_positions = np.array([[0, 1, 0], [2, 1, 0], [-2, 1, 0]]) - def _create_flat_bank_info(self, use_relative_positions: bool, detector_ids: list[int] = [5, 10, 15]) -> FlatBankInfo: + def _create_flat_bank_info(self, use_relative_positions: bool, detector_ids: list[int] | None = None) -> FlatBankInfo: flat_bank = FlatBankInfo() flat_bank.reference_position = np.zeros(3) - flat_bank.detector_ids = detector_ids + flat_bank.detector_ids = detector_ids if detector_ids is not None else [5, 10, 15] if use_relative_positions: flat_bank.relative_projected_positions = self.detector_positions.copy() else: diff --git a/qt/python/instrumentview/instrumentview/test/test_model.py b/qt/python/instrumentview/instrumentview/test/test_model.py index c4c5e5c8d851..5e1cdd5ddf5a 100644 --- a/qt/python/instrumentview/instrumentview/test/test_model.py +++ b/qt/python/instrumentview/instrumentview/test/test_model.py @@ -132,6 +132,10 @@ def test_calculate_spherical_projection(self, mock_spherical_projection): def test_calculate_cylindrical_projection(self, mock_cylindrical_projection): self._run_projection_test(mock_cylindrical_projection, FullInstrumentViewModel._CYLINDRICAL_Y) + @mock.patch("instrumentview.Projections.SideBySide.SideBySide") + def test_calculate_side_by_side_projection(self, mock_side_by_side): + self._run_projection_test(mock_side_by_side, FullInstrumentViewModel._SIDE_BY_SIDE) + def _run_projection_test(self, mock_projection_constructor, projection_option): self._mock_detector_table([1, 2, 3]) mock_workspace = self._create_mock_workspace([1, 2, 3]) From 2643e991ee272cc87402c017b5896c705c558672 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Wed, 24 Sep 2025 09:34:46 +0100 Subject: [PATCH 22/23] Test fix and tidy up Mock method in a test was returning the wrong type of object, and did a couple of improvements from code review. --- Framework/API/src/PanelsSurfaceCalculator.cpp | 4 ++++ .../instrumentview/instrumentview/Projections/Projection.py | 2 +- .../instrumentview/Projections/test/test_SideBySide.py | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Framework/API/src/PanelsSurfaceCalculator.cpp b/Framework/API/src/PanelsSurfaceCalculator.cpp index 496b4b8866a8..adba3f3825ad 100644 --- a/Framework/API/src/PanelsSurfaceCalculator.cpp +++ b/Framework/API/src/PanelsSurfaceCalculator.cpp @@ -396,6 +396,10 @@ std::optional PanelsSurfaceCalculator::getSideBySideViewPos(const C const size_t componentIndex) const { const auto *componentID = componentInfo.componentID(componentIndex); const auto component = instrument->getComponentByID(componentID); + if (!component) { + return std::optional(); + } + return component->getSideBySideViewPos(); } diff --git a/qt/python/instrumentview/instrumentview/Projections/Projection.py b/qt/python/instrumentview/instrumentview/Projections/Projection.py index 0aa4cc70895e..08d9f83238eb 100644 --- a/qt/python/instrumentview/instrumentview/Projections/Projection.py +++ b/qt/python/instrumentview/instrumentview/Projections/Projection.py @@ -35,7 +35,7 @@ def __init__( self._u_period = 2 * np.pi - self._calculate_axes(root_position) + self._calculate_axes(self._root_position) self._calculate_detector_coordinates() self._find_and_correct_x_gap() diff --git a/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py b/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py index deb2387280f9..ed370b380d20 100644 --- a/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py +++ b/qt/python/instrumentview/instrumentview/Projections/test/test_SideBySide.py @@ -120,7 +120,7 @@ def test_construct_flat_panels( side_by_side = self._create_side_by_side([5, 10, 15, 20, 25], True) mock_construct_rectangles.return_value = [self._create_flat_bank_info(True, detector_ids=[5, 10, 15])] mock_construct_tube_banks.return_value = [self._create_flat_bank_info(True, detector_ids=[20, 25])] - mock_create_flat_bank.return_value = [] + mock_create_flat_bank.return_value = None side_by_side._construct_flat_panels(side_by_side._workspace) detector_map = side_by_side._detector_id_to_flat_bank_map self.assertEqual([5, 10, 15, 20, 25], list(detector_map.keys())) From 29b6bfec761e7505cf18c910b220a752a4186c95 Mon Sep 17 00:00:00 2001 From: James Clarke Date: Wed, 24 Sep 2025 13:39:40 +0100 Subject: [PATCH 23/23] Fix compiler warnings boost::python::len returns a signed int, so we need to convert to a size_t before comparisons in the loops. boost::python::extract generates a few maybe-uninitialized warnings with gcc. --- Framework/API/src/PanelsSurfaceCalculator.cpp | 1 - .../src/Exports/PanelsSurfaceCalculator.cpp | 18 +++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Framework/API/src/PanelsSurfaceCalculator.cpp b/Framework/API/src/PanelsSurfaceCalculator.cpp index adba3f3825ad..f8819a48192f 100644 --- a/Framework/API/src/PanelsSurfaceCalculator.cpp +++ b/Framework/API/src/PanelsSurfaceCalculator.cpp @@ -4,7 +4,6 @@ // NScD Oak Ridge National Laboratory, European Spallation Source, // Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS // SPDX - License - Identifier: GPL - 3.0 + -#pragma once #include "MantidAPI/PanelsSurfaceCalculator.h" #include "MantidAPI/MatrixWorkspace.h" diff --git a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp index 9f650ff7b94e..d913a14c7b48 100644 --- a/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp +++ b/Framework/PythonInterface/mantid/api/src/Exports/PanelsSurfaceCalculator.cpp @@ -6,10 +6,14 @@ // SPDX - License - Identifier: GPL - 3.0 + #include "MantidAPI/PanelsSurfaceCalculator.h" #include "MantidAPI/MatrixWorkspace.h" +#include "MantidKernel/WarningSuppressions.h" #include #include #include +// boost::python generates this warning in gcc +GNU_DIAG_OFF("maybe-uninitialized") + using namespace boost::python; namespace { @@ -63,7 +67,8 @@ bool isBankFlat(PanelsSurfaceCalculator &self, const object &componentInfo, cons const list &normal) { const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); std::vector tubesVector; - for (size_t i = 0; i < len(tubes); i++) { + const size_t lenTubes = len(tubes); + for (size_t i = 0; i < lenTubes; i++) { tubesVector.push_back(extract(tubes[i])); } const auto normalV3D = pythonListToV3D(normal); @@ -73,7 +78,8 @@ bool isBankFlat(PanelsSurfaceCalculator &self, const object &componentInfo, cons list calculateBankNormal(PanelsSurfaceCalculator &self, const object &componentInfo, const list &tubes) { const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); std::vector tubesVector; - for (size_t i = 0; i < len(tubes); i++) { + const size_t lenTubes = len(tubes); + for (size_t i = 0; i < lenTubes; i++) { tubesVector.push_back(extract(tubes[i])); } const auto normal = self.calculateBankNormal(*(cInfoSharedPtr.get()), tubesVector); @@ -85,11 +91,12 @@ void setBankVisited(const PanelsSurfaceCalculator &self, const object &component list &visitedComponents) { const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); std::vector visitedComponentsVector; - for (size_t i = 0; i < len(visitedComponents); i++) { + const size_t lenVisitedComponents = len(visitedComponents); + for (size_t i = 0; i < lenVisitedComponents; i++) { visitedComponentsVector.push_back(extract(visitedComponents[i])); } self.setBankVisited(*(cInfoSharedPtr.get()), bankIndex, visitedComponentsVector); - for (size_t i = 0; i < len(visitedComponents); i++) { + for (size_t i = 0; i < lenVisitedComponents; i++) { // A bool is 8 bytes because a byte is the smallest addressable unit of memory, but // an std::vector uses bits to store bools as a memory optimisation. Hence // the objects in a vector are not actually bools, and boost::python @@ -101,7 +108,8 @@ void setBankVisited(const PanelsSurfaceCalculator &self, const object &component size_t findNumDetectors(const PanelsSurfaceCalculator &self, const object &componentInfo, const list &components) { const std::shared_ptr cInfoSharedPtr = extract>(componentInfo); std::vector componentsVector; - for (size_t i = 0; i < len(components); i++) { + const size_t lenComponents = len(components); + for (size_t i = 0; i < lenComponents; i++) { componentsVector.push_back(extract(components[i])); } return self.findNumDetectors(*(cInfoSharedPtr.get()), componentsVector);