From 3596fcbe898ea33d49b1380a6fa6ef34f95b12e7 Mon Sep 17 00:00:00 2001
From: Karl Palmen <karl.palmen@stfc.ac.uk>
Date: Tue, 20 Dec 2011 15:02:34 +0000
Subject: [PATCH] Make Q1D2 fail, if non-positive PixelAdj is unmasked re #4321

Also add unit test for this.
Firstly require failure, if two pixels with PixelAdj values of 0 and -1, have unmasked detectors.
Secondly require success if those two detectors are masked.
A test for all PixelAdj values positive already exists.

Signed-off-by: Karl Palmen <karl.palmen@stfc.ac.uk>
---
 .../Framework/Algorithms/src/Qhelper.cpp      | 23 +++++-
 .../Framework/Algorithms/test/Q1D2Test.h      | 72 ++++++++++++++++---
 .../Framework/Geometry/src/Objects/Object.cpp |  5 +-
 3 files changed, 89 insertions(+), 11 deletions(-)

diff --git a/Code/Mantid/Framework/Algorithms/src/Qhelper.cpp b/Code/Mantid/Framework/Algorithms/src/Qhelper.cpp
index 5bdd7f90d8a..0035ecfb80f 100644
--- a/Code/Mantid/Framework/Algorithms/src/Qhelper.cpp
+++ b/Code/Mantid/Framework/Algorithms/src/Qhelper.cpp
@@ -24,8 +24,8 @@ using namespace Geometry;
 
 /** Checks if workspaces input to Q1D or Qxy are reasonable
   @param dataWS data workspace
-  @param binWS (WavelengthAdj) workpace that will be checked to see if it has one spectrum and the same number of bins as dataWS
-  @param detectWS (PixelAdj) passing NULL for this wont raise an error, if set it will be checked this workspace has as many histograms as dataWS each with one bin
+  @param binAdj (WavelengthAdj) workpace that will be checked to see if it has one spectrum and the same number of bins as dataWS
+  @param detectAdj (PixelAdj) passing NULL for this wont raise an error, if set it will be checked this workspace has as many histograms as dataWS each with one bin
   @throw invalid_argument if the workspaces are not mututially compatible
 */
 void Qhelper::examineInput(API::MatrixWorkspace_const_sptr dataWS, 
@@ -66,6 +66,8 @@ void Qhelper::examineInput(API::MatrixWorkspace_const_sptr dataWS,
     //throw std::invalid_argument("The data workspace must be a distrbution if there is no Wavelength dependent adjustment");
   }
   
+  // Perform tests on detectAdj
+
   if (detectAdj)
   {
     if ( detectAdj->blocksize() != 1 )
@@ -76,6 +78,23 @@ void Qhelper::examineInput(API::MatrixWorkspace_const_sptr dataWS,
     {
       throw std::invalid_argument("The PixelAdj workspace must have one spectrum for each spectrum in the detector bank workspace");
     }
+
+    // test that when detector adjustment value less than or equal to zero that the corresponding detector 
+    // in the workspace is masked
+
+    size_t num_histograms = dataWS->getNumberHistograms();
+    for( size_t i=0; i<num_histograms; i++ )
+    {
+      double adj = (double)detectAdj->readY(i)[0];
+      if( adj <= 0.0) 
+      {
+        if( ! dataWS->getDetector(i)->isMasked())
+        {
+          throw std::invalid_argument ("Every detector with non-positive PixelAdj value must be masked");
+        }
+      }
+    }
+
   }
 }
 
diff --git a/Code/Mantid/Framework/Algorithms/test/Q1D2Test.h b/Code/Mantid/Framework/Algorithms/test/Q1D2Test.h
index fb93c42f075..35e3d16cdcb 100644
--- a/Code/Mantid/Framework/Algorithms/test/Q1D2Test.h
+++ b/Code/Mantid/Framework/Algorithms/test/Q1D2Test.h
@@ -8,6 +8,7 @@
 #include "MantidAlgorithms/CropWorkspace.h"
 #include "MantidDataHandling/LoadRaw3.h"
 #include "MantidDataHandling/LoadRKH.h"
+#include "MantidDataHandling/MaskDetectors.h"
 #include <boost/math/special_functions/fpclassify.hpp>
 
 #include "MantidTestHelpers/WorkspaceCreationHelper.h"
@@ -22,6 +23,7 @@ static double flat_cell061Es [] = {8.140295E-03, 8.260089E-03, 8.198814E-03, 8.3
 
 /// defined below this creates some input data
 void createInputWorkspaces(int start, int end, Mantid::API::MatrixWorkspace_sptr & input, Mantid::API::MatrixWorkspace_sptr & wave, Mantid::API::MatrixWorkspace_sptr & pixels);
+void createInputWorkSpacesForMasking ( Mantid::API::MatrixWorkspace_sptr & input, Mantid::API::MatrixWorkspace_sptr & wave, Mantid::API::MatrixWorkspace_sptr & pixels);
 
 class Q1D2Test : public CxxTest::TestSuite
 {
@@ -118,6 +120,41 @@ public:
     
   }
 
+  void testInvalidPixelAdj()
+  {
+    Mantid::API::MatrixWorkspace_sptr mask_input, mask_wave, mask_pixels;
+    createInputWorkSpacesForMasking( mask_input, mask_wave, mask_pixels);
+
+    Mantid::DataHandling::MaskDetectors maskDetectors;
+    Mantid::Algorithms::Q1D2 Q1D;
+    Q1D.initialize();
+
+    // First run algorithm on workspaces for masking without masking any pixels. 
+    // It should fail because first two pixels have non-positive PixelAdj values
+    const std::string outputWS("Q1D2Test_invalid_Pixel_Adj");
+    TS_ASSERT_THROWS_NOTHING(
+      Q1D.setProperty("DetBankWorkspace", mask_input);
+      Q1D.setProperty("WavelengthAdj", mask_wave);
+      Q1D.setProperty("PixelAdj", mask_pixels);
+      Q1D.setPropertyValue("OutputWorkspace", outputWS);
+      Q1D.setPropertyValue("OutputBinning", "0.1,-0.02,0.5");
+    )
+    Q1D.execute();
+
+    TS_ASSERT( ! Q1D.isExecuted() )
+
+    // Secondly we mask the detectors for these two pixels
+    maskDetectors.initialize();
+    maskDetectors.setPropertyValue("Workspace","Q1D2Test_inputworkspace_for_masking");;
+    maskDetectors.setPropertyValue("WorkspaceIndexList","5,6");
+    maskDetectors.execute();
+
+    // Now it should work
+    TS_ASSERT_THROWS_NOTHING( Q1D.execute() )
+    TS_ASSERT( Q1D.isExecuted() )
+
+  }
+
   void testGravity()
   {
     Mantid::Algorithms::Q1D2 Q1D;
@@ -252,24 +289,25 @@ public:
   {
     Mantid::Algorithms::Q1D2 Q1D;
     Q1D.initialize();
-    
+
     //this is a small change to the normalization workspace that should be enough to stop progress
     Mantid::MantidVec & xData = m_wavNorm->dataX(0);
     xData[15] += 0.001;
 
     const std::string outputWS("Q1D2Test_invalid_result");
     TS_ASSERT_THROWS_NOTHING(
-      Q1D.setProperty("DetBankWorkspace", m_inputWS);
-      Q1D.setProperty("WavelengthAdj", m_wavNorm);
-      Q1D.setPropertyValue("OutputWorkspace", outputWS);
-      Q1D.setPropertyValue("OutputBinning", "0.1,-0.02,0.5");
-      Q1D.setPropertyValue("AccountForGravity", "1");
+    Q1D.setProperty("DetBankWorkspace", m_inputWS);
+    Q1D.setProperty("WavelengthAdj", m_wavNorm);
+    Q1D.setPropertyValue("OutputWorkspace", outputWS);
+    Q1D.setPropertyValue("OutputBinning", "0.1,-0.02,0.5");
+    Q1D.setPropertyValue("AccountForGravity", "1");
     )
+      Q1D.execute();
 
     TS_ASSERT( ! Q1D.isExecuted() )
   }
   
-  ///stop the constructor from being run every time algorithms test suit is initialised
+  ///stop the constructor from being run every time algorithms test suite is initialised
   static Q1D2Test *createSuite() { return new Q1D2Test(); }
   static void destroySuite(Q1D2Test *suite) { delete suite; }
   Q1D2Test() : m_noGrav("Q1D2Test_no_gravity_result")
@@ -318,7 +356,15 @@ public:
 
 void createInputWorkspaces(int start, int end, Mantid::API::MatrixWorkspace_sptr & input, Mantid::API::MatrixWorkspace_sptr & wave, Mantid::API::MatrixWorkspace_sptr & pixels)
 {
+  
   std::string wsName("Q1D2Test_inputworkspace"), wavNorm("Q1D2Test_wave");
+  
+  bool forMasking = (start > 9000); // If start is late we assume we a creating a 2nd set of workspaces for masking
+  if( forMasking) // avoid workspace name clash if creating workspaces for masking
+  {
+    wsName = "Q1D2Test_inputworkspace_for_masking", 
+    wavNorm = "Q1D2Test_wave_for_masking";
+  }
 
   LoadRaw3 loader;
   loader.initialize();
@@ -367,7 +413,17 @@ void createInputWorkspaces(int start, int end, Mantid::API::MatrixWorkspace_sptr
     pixels->dataY(i)[0] = flat_cell061Ys[i];
     pixels->dataE(i)[0] = flat_cell061Es[i];
   }
-  AnalysisDataService::Instance().add("Q1DTest_flat_file", pixels);
+  if ( forMasking ) 
+  {
+      pixels->dataY(5)[0] = 0.0;  // This pixels should be masked
+      pixels->dataY(6)[0] = -1.0;  // This pixel should be masked
+      AnalysisDataService::Instance().add("Q1DTest_flat_file", pixels);
+  } 
+  else AnalysisDataService::Instance().add("Q1DTest_flat_file_for_masking", pixels);
     
 }
+void createInputWorkSpacesForMasking ( Mantid::API::MatrixWorkspace_sptr & input, Mantid::API::MatrixWorkspace_sptr & wave, Mantid::API::MatrixWorkspace_sptr & pixels)
+{
+   createInputWorkspaces ( 9001, 9030, input, wave, pixels );
+}
 #endif /*Q1D2Test_H_*/
diff --git a/Code/Mantid/Framework/Geometry/src/Objects/Object.cpp b/Code/Mantid/Framework/Geometry/src/Objects/Object.cpp
index af14f3f3deb..babe1336da2 100644
--- a/Code/Mantid/Framework/Geometry/src/Objects/Object.cpp
+++ b/Code/Mantid/Framework/Geometry/src/Objects/Object.cpp
@@ -1618,7 +1618,10 @@ namespace Mantid
       AABBzMin = zMin;
       boolBounded = true;
 
-      m_boundingBox = BoundingBox(xMax, yMax, zMax, xMin, yMin, zMin);
+      PARALLEL_CRITICAL(defineBoundingBox)
+      {
+        m_boundingBox = BoundingBox(xMax, yMax, zMax, xMin, yMin, zMin);
+      }
     }
 
     /**
-- 
GitLab