Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix logic for checking bin consistency in ConjoinWorkspaces algorithm #38619

Merged
merged 5 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class MANTID_ALGORITHMS_DLL ConjoinWorkspaces : public WorkspaceJoiners {
void init() override;
void exec() override;

bool checkBinning(const API::MatrixWorkspace_const_sptr &ws1, const API::MatrixWorkspace_const_sptr &ws2) const;
void checkForOverlap(const API::MatrixWorkspace &ws1, const API::MatrixWorkspace &ws2, bool checkSpectra) const;
API::MatrixWorkspace_sptr conjoinEvents(const DataObjects::EventWorkspace &ws1,
const DataObjects::EventWorkspace &ws2);
Expand Down
4 changes: 2 additions & 2 deletions Framework/Algorithms/src/AppendSpectra.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ void AppendSpectra::exec() {
if (((eventWs1) && (!eventWs2)) || ((!eventWs1) && (eventWs2))) {
const std::string message("Only one of the input workspaces are of type "
"EventWorkspace; please use matching workspace "
"types (both EventWorkspace's or both "
"Workspace2D's).");
"types (both EventWorkspace or both "
"Workspace2D).");
g_log.error(message);
throw std::invalid_argument(message);
}
Expand Down
27 changes: 23 additions & 4 deletions Framework/Algorithms/src/ConjoinWorkspaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,15 @@ void ConjoinWorkspaces::exec() {
if (((eventWs1) && (!eventWs2)) || ((!eventWs1) && (eventWs2))) {
const std::string message("Only one of the input workspaces are of type "
"EventWorkspace; please use matching workspace "
"types (both EventWorkspace's or both "
"Workspace2D's).");
"types (both EventWorkspace or both "
"Workspace2D).");
g_log.error(message);
throw std::invalid_argument(message);
}
// Check if bins match

// Check whether bins match
bool checkBins = getProperty("CheckMatchingBins");
if (!WorkspaceHelpers::matchingBins(ws1, ws2) && checkBins) {
if (checkBins && !checkBinning(ws1, ws2)) {
const std::string message("The bins do not match in the input workspaces. "
"Consider using RebinToWorkspace to preprocess "
"the workspaces before conjoining them.");
Expand All @@ -88,6 +89,24 @@ void ConjoinWorkspaces::exec() {
AnalysisDataService::Instance().remove(getPropertyValue("InputWorkspace2"));
}

//----------------------------------------------------------------------------------------------
/** Checks whether the binning is consistent between two workspaces
* @param ws1 :: The first input workspace
* @param ws2 :: The second input workspace
* @return :: true if both workspaces have consistent binning.
*/
bool ConjoinWorkspaces::checkBinning(const API::MatrixWorkspace_const_sptr &ws1,
const API::MatrixWorkspace_const_sptr &ws2) const {
if (ws1->isRaggedWorkspace() || ws2->isRaggedWorkspace()) {
return false;
}

// If neither workspace is ragged, we only need to check the first specrum.
// Otherwise the matchingBins() function requires the two workspaces to have
// the same number of spectra.
return WorkspaceHelpers::matchingBins(ws1, ws2, true);
}

//----------------------------------------------------------------------------------------------
/** Checks that the two input workspaces have non-overlapping spectra numbers
* and contributing detectors
Expand Down
3 changes: 1 addition & 2 deletions Framework/Algorithms/src/WorkspaceJoiners.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,7 @@ DataObjects::EventWorkspace_sptr WorkspaceJoiners::execEvent(const DataObjects::
return output;
}

/** Checks that the two input workspace have common size and the same
* instrument & unit.
/** Checks that the two input workspace have the same instrument, unit and distribution flag.
* @param ws1 :: The first input workspace
* @param ws2 :: The second input workspace
* @throw std::invalid_argument If the workspaces are not compatible
Expand Down
19 changes: 17 additions & 2 deletions Framework/Algorithms/test/ConjoinWorkspacesTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ class ConjoinWorkspacesTest : public CxxTest::TestSuite {
TS_ASSERT(!conj.isExecuted());
}

void testCheckMatchingBinsError() {
void testNonMatchingBinsThrowsError() {
MatrixWorkspace_sptr ws1, ws2;
ws1 = WorkspaceCreationHelper::createEventWorkspace(10, 5);
ws2 = WorkspaceCreationHelper::createEventWorkspace(10, 10);
Expand All @@ -192,6 +192,22 @@ class ConjoinWorkspacesTest : public CxxTest::TestSuite {
}
}

void testMatchingBinsThrowsNoError() {
MatrixWorkspace_sptr ws1 = WorkspaceCreationHelper::createEventWorkspace(10, 5);
MatrixWorkspace_sptr ws2 = WorkspaceCreationHelper::createEventWorkspace(10, 5);

// CheckMatchingBins is true by default, so don't set it here.
ConjoinWorkspaces conj;
conj.initialize();
conj.setProperty("CheckOverlapping", false);
conj.setProperty("InputWorkspace1", ws1);
conj.setProperty("InputWorkspace2", ws2);
conj.setAlwaysStoreInADS(false);
conj.setRethrows(true);

TS_ASSERT_THROWS_NOTHING(conj.execute());
}

void testDoCheckForOverlap() {
MatrixWorkspace_sptr ws1, ws2;
int numPixels = 10;
Expand Down Expand Up @@ -412,7 +428,6 @@ class ConjoinWorkspacesTest : public CxxTest::TestSuite {
MatrixWorkspace_sptr ews = WorkspaceCreationHelper::createEventWorkspace(10, 10);
AnalysisDataService::Instance().addOrReplace(name, ews);

// Crop ews to have first 3 spectra, ews2 to have second 3
CropWorkspace crop;
crop.setChild(true);
crop.initialize();
Expand Down
Loading