From 0c2cfdf8294a116920a04d4adf85dd300590a923 Mon Sep 17 00:00:00 2001 From: Kort Travis Date: Mon, 28 Oct 2024 17:33:35 -0400 Subject: [PATCH] LoadNexusProcessed: root-level NXentry groups Mantid's interpretation of the NeXus format uses multiple root-level "NX_class: NXentry" groups to store multiple workspaces in a single NeXus HDF5 file. By using additional root-level groups, such as "NX_class: NXcollection", metadata about a group of workspaces may be stored. In addition, the NeXus format itself allows other, non-NeXus information to be stored in an HDF5 file, provided it avoids collisions with the NeXus naming system. In combination, all of this led to the requirement solved by the current PR, which allows `LoadNexusProcessed` to function correctly in the presence of other root-level non-NXentry and non-NeXus groups. This commit includes the following changes: * When `LoadNexusProcessed` is determining the number of workspaces in a NeXus HDF5 file, it now counts the number of root-level "NX_class: NXentry" groups. Previously, it counted the number of root-level groups, assuming all were of "NX_class: NXentry". --- .../DataHandling/src/LoadNexusProcessed.cpp | 25 +++++++++-------- .../test/LoadNexusProcessedTest.h | 28 +++++++++++++++++++ .../WorkspaceGroup_other_groups.nxs.md5 | 1 + .../CMake/CppCheck_Suppressions.txt.in | 1 - .../Framework/Algorithms/Bugfixes/38324.rst | 1 + 5 files changed, 43 insertions(+), 13 deletions(-) create mode 100644 Testing/Data/UnitTest/WorkspaceGroup_other_groups.nxs.md5 create mode 100644 docs/source/release/v6.12.0/Framework/Algorithms/Bugfixes/38324.rst diff --git a/Framework/DataHandling/src/LoadNexusProcessed.cpp b/Framework/DataHandling/src/LoadNexusProcessed.cpp index 88984a694777..7b80e1bf24a2 100644 --- a/Framework/DataHandling/src/LoadNexusProcessed.cpp +++ b/Framework/DataHandling/src/LoadNexusProcessed.cpp @@ -40,6 +40,7 @@ #include #include +#include #include #include #include @@ -373,28 +374,28 @@ Workspace_sptr LoadNexusProcessed::doAccelleratedMultiPeriodLoading(NXRoot &root void LoadNexusProcessed::execLoader() { API::Workspace_sptr tempWS; - int nWorkspaceEntries = 0; + size_t nWorkspaceEntries = 0; // Start scoped block { progress(0, "Opening file..."); - // Throws an approriate exception if there is a problem with file access + // Throws an appropriate exception if there is a problem with file access const std::string filename = getPropertyValue("Filename"); NXRoot root(filename); // "Open" the same file but with the C++ interface m_nexusFile = std::make_unique<::NeXus::File>(root.m_fileID); - // Find out how many first level entries there are - // Cast down to int as another property later on is an int - nWorkspaceEntries = static_cast((root.groups().size())); + // Find out how many NXentry groups there are in the file. + nWorkspaceEntries = std::count_if(root.groups().cbegin(), root.groups().cend(), + [](const auto &g) { return g.nxclass == "NXentry"; }); // Check for an entry number property int entrynumber = getProperty("EntryNumber"); Property const *const entryNumberProperty = this->getProperty("EntryNumber"); bool bDefaultEntryNumber = entryNumberProperty->isDefault(); - if (!bDefaultEntryNumber && entrynumber > nWorkspaceEntries) { + if (!bDefaultEntryNumber && entrynumber > static_cast(nWorkspaceEntries)) { g_log.error() << "Invalid entry number specified. File only contains " << nWorkspaceEntries << " entries.\n"; throw std::invalid_argument("Invalid entry number specified."); } @@ -421,7 +422,7 @@ void LoadNexusProcessed::execLoader() { // We already know that this is a group workspace. Is it a true // multiperiod workspace. const bool bFastMultiPeriod = this->getProperty("FastMultiPeriod"); - const bool bIsMultiPeriod = isMultiPeriodFile(nWorkspaceEntries, tempWS, g_log); + const bool bIsMultiPeriod = isMultiPeriodFile(static_cast(nWorkspaceEntries), tempWS, g_log); const Property *specListProp = this->getProperty("SpectrumList"); m_list = !specListProp->isDefault(); @@ -433,7 +434,7 @@ void LoadNexusProcessed::execLoader() { // load names of each of the workspaces. Note that if we have duplicate // names then we don't select them - auto names = extractWorkspaceNames(root, static_cast(nWorkspaceEntries)); + auto names = extractWorkspaceNames(root, nWorkspaceEntries); // remove existing workspace and replace with the one being loaded bool wsExists = AnalysisDataService::Instance().doesExist(base_name); @@ -465,7 +466,7 @@ void LoadNexusProcessed::execLoader() { g_log.information("Individual group loading"); } - for (int p = 1; p <= nWorkspaceEntries; ++p) { + for (size_t p = 1; p <= nWorkspaceEntries; ++p) { const auto indexStr = std::to_string(p); // decide what the workspace should be called @@ -503,7 +504,7 @@ void LoadNexusProcessed::execLoader() { root.close(); } // All file resources should be scoped to here. All previous file handles // must be cleared to release locks - loadNexusGeometry(*tempWS, nWorkspaceEntries, g_log, std::string(getProperty("Filename"))); + loadNexusGeometry(*tempWS, static_cast(nWorkspaceEntries), g_log, std::string(getProperty("Filename"))); m_axis1vals.clear(); } // namespace DataHandling @@ -913,13 +914,13 @@ void LoadNexusProcessed::loadV3DColumn(Mantid::NeXus::NXDouble &data, const API: const int rowCount = data.dim0(); - // This might've been done already, but doing it twice should't do any harm + // This might've been done already, but doing it twice shouldn't do any harm tableWs->setRowCount(rowCount); data.load(); for (int i = 0; i < rowCount; ++i) { - auto &cell = col[i]; + auto &cell = col[i]; // cppcheck-suppress constVariableReference cell(data(i, 0), data(i, 1), data(i, 2)); } } diff --git a/Framework/DataHandling/test/LoadNexusProcessedTest.h b/Framework/DataHandling/test/LoadNexusProcessedTest.h index c2af7918d65f..a8ec2fafae72 100644 --- a/Framework/DataHandling/test/LoadNexusProcessedTest.h +++ b/Framework/DataHandling/test/LoadNexusProcessedTest.h @@ -539,6 +539,34 @@ class LoadNexusProcessedTest : public CxxTest::TestSuite { } } + void test_load_workspace_group_other_root_groups() { + + // Test that a group workspace can be loaded in the presence of other + // non-NXentry NeXus and non-NeXus root groups. + + LoadNexusProcessed alg; + TS_ASSERT_THROWS_NOTHING(alg.initialize()); + TS_ASSERT(alg.isInitialized()); + alg.setPropertyValue("Filename", "WorkspaceGroup_other_groups.nxs"); + alg.setPropertyValue("OutputWorkspace", "group"); + + TS_ASSERT_THROWS_NOTHING(alg.execute()); + + Workspace_sptr workspace; + TS_ASSERT_THROWS_NOTHING(workspace = AnalysisDataService::Instance().retrieve("group")); + WorkspaceGroup_sptr group = std::dynamic_pointer_cast(workspace); + TS_ASSERT(group); + int groupSize = group->getNumberOfEntries(); + TS_ASSERT_EQUALS(groupSize, 12); + for (int i = 0; i < groupSize; ++i) { + MatrixWorkspace_sptr ws = std::dynamic_pointer_cast(group->getItem(i)); + TS_ASSERT(ws); + TS_ASSERT_EQUALS(ws->getNumberHistograms(), 1); + TS_ASSERT_EQUALS(ws->blocksize(), 10); + TS_ASSERT_EQUALS(ws->getName(), "group_" + std::to_string(i + 1)); + } + } + void test_load_fit_parameters() { LoadNexusProcessed alg; TS_ASSERT_THROWS_NOTHING(alg.initialize()); diff --git a/Testing/Data/UnitTest/WorkspaceGroup_other_groups.nxs.md5 b/Testing/Data/UnitTest/WorkspaceGroup_other_groups.nxs.md5 new file mode 100644 index 000000000000..a6c5146b1561 --- /dev/null +++ b/Testing/Data/UnitTest/WorkspaceGroup_other_groups.nxs.md5 @@ -0,0 +1 @@ +cc76a6f1e3a36626a1d38cda0f88e5ac diff --git a/buildconfig/CMake/CppCheck_Suppressions.txt.in b/buildconfig/CMake/CppCheck_Suppressions.txt.in index b33e69618faa..0b38ed5bb017 100644 --- a/buildconfig/CMake/CppCheck_Suppressions.txt.in +++ b/buildconfig/CMake/CppCheck_Suppressions.txt.in @@ -733,7 +733,6 @@ constVariableReference:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/SaveFullpr passedByValue:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveHKL.cpp:579 passedByValue:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveHKL.cpp:580 constVariableReference:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveHKL.cpp:158 -constVariableReference:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/LoadNexusProcessed.cpp:922 containerOutOfBounds:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveHKL.cpp:607 constVariableReference:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveLauenorm.cpp:164 constVariableReference:${CMAKE_SOURCE_DIR}/Framework/Crystal/src/SaveLauenorm.cpp:234 diff --git a/docs/source/release/v6.12.0/Framework/Algorithms/Bugfixes/38324.rst b/docs/source/release/v6.12.0/Framework/Algorithms/Bugfixes/38324.rst new file mode 100644 index 000000000000..86a61700ee10 --- /dev/null +++ b/docs/source/release/v6.12.0/Framework/Algorithms/Bugfixes/38324.rst @@ -0,0 +1 @@ +- fixes a bug in :ref:`LoadNexusProcessed ` when determining the number of workspaces in a NeXus HDF5 file. It now counts the number of root-level "NX_class: NXentry" groups. Previously, it simply counted the number of root-level groups, assuming all were of "NX_class: NXentry".