From a6a122b3bd26258b10655974b8dfad94a08279a8 Mon Sep 17 00:00:00 2001 From: Karl Palmen Date: Thu, 5 Dec 2013 15:51:09 +0000 Subject: [PATCH 1/8] Add non-active parameterXML property re #8546 I also drilled it down into the execManually method changing its arguments. Signed-off-by: Karl Palmen --- .../inc/MantidDataHandling/LoadParameterFile.h | 2 +- .../DataHandling/src/LoadIDFFromNexus.cpp | 2 +- .../DataHandling/src/LoadInstrument.cpp | 2 +- .../DataHandling/src/LoadParameterFile.cpp | 16 ++++++++++------ 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/Code/Mantid/Framework/DataHandling/inc/MantidDataHandling/LoadParameterFile.h b/Code/Mantid/Framework/DataHandling/inc/MantidDataHandling/LoadParameterFile.h index 83147785439b..1a0bad10c7f7 100644 --- a/Code/Mantid/Framework/DataHandling/inc/MantidDataHandling/LoadParameterFile.h +++ b/Code/Mantid/Framework/DataHandling/inc/MantidDataHandling/LoadParameterFile.h @@ -84,7 +84,7 @@ namespace Mantid /// Algorithm's category for identification overriding a virtual method virtual const std::string category() const { return "DataHandling\\Instrument";} - static void execManually(std::string filename, Mantid::API::ExperimentInfo_sptr localWorkspace); + static void execManually(bool useString, std::string filename, std::string parameterString, Mantid::API::ExperimentInfo_sptr localWorkspace); private: /// Sets documentation strings for this algorithm diff --git a/Code/Mantid/Framework/DataHandling/src/LoadIDFFromNexus.cpp b/Code/Mantid/Framework/DataHandling/src/LoadIDFFromNexus.cpp index 9e81ce5b3132..63188bc98028 100644 --- a/Code/Mantid/Framework/DataHandling/src/LoadIDFFromNexus.cpp +++ b/Code/Mantid/Framework/DataHandling/src/LoadIDFFromNexus.cpp @@ -90,7 +90,7 @@ void LoadIDFFromNexus::runLoadParameterFile(const MatrixWorkspace_sptr & workspa const std::string paramFile = directory + instrumentName + "_Parameters.xml"; try { - LoadParameterFile::execManually(paramFile, workspace); + LoadParameterFile::execManually(false, paramFile,"", workspace); } catch ( std::runtime_error& ) { g_log.notice() << "File " << paramFile << " not found or un-parsable. " "However, the instrument has been loaded successfully.\n"; diff --git a/Code/Mantid/Framework/DataHandling/src/LoadInstrument.cpp b/Code/Mantid/Framework/DataHandling/src/LoadInstrument.cpp index f79f227b0f30..4aec499f0b65 100644 --- a/Code/Mantid/Framework/DataHandling/src/LoadInstrument.cpp +++ b/Code/Mantid/Framework/DataHandling/src/LoadInstrument.cpp @@ -237,7 +237,7 @@ namespace Mantid try { // To allow the use of ExperimentInfo instead of workspace, we call it manually - LoadParameterFile::execManually(fullPathParamIDF, m_workspace); + LoadParameterFile::execManually(false, fullPathParamIDF, "", m_workspace); g_log.debug("Parameters loaded successfully."); } catch (std::invalid_argument& e) { diff --git a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp index fa30cca9b7b5..6ec3424405a2 100644 --- a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp +++ b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp @@ -59,8 +59,8 @@ DECLARE_ALGORITHM(LoadParameterFile) /// Sets documentation strings for this algorithm void LoadParameterFile::initDocs() { - this->setWikiSummary("Loads instrument parameters into a [[workspace]]. where these parameters are associated component names as defined in Instrument Definition File ([[InstrumentDefinitionFile|IDF]])."); - this->setOptionalMessage("Loads instrument parameters into a workspace. where these parameters are associated component names as defined in Instrument Definition File (IDF)."); + this->setWikiSummary("Loads instrument parameters into a [[workspace]]. where these parameters are associated component names as defined in Instrument Definition File ([[InstrumentDefinitionFile|IDF]]) or a string consisting of the contents of such.."); + this->setOptionalMessage("Loads instrument parameters into a workspace. where these parameters are associated component names as defined in Instrument Definition File (IDF) or a string consisting of the contents of such."); } @@ -80,8 +80,9 @@ void LoadParameterFile::init() declareProperty( new WorkspaceProperty("Workspace","Anonymous",Direction::InOut), "The name of the workspace to load the instrument parameters into." ); - declareProperty(new FileProperty("Filename","", FileProperty::Load, ".xml"), - "The filename (including its full or relative path) of a parameter defintion file. The file extension must either be .xml or .XML."); + declareProperty(new FileProperty("Filename","", FileProperty::OptionalLoad, ".xml"), + "The filename (including its full or relative path) of a parameter definition file. The file extension must either be .xml or .XML."); + declareProperty("ParameterXML","","The parameter definition XML as a string."); } /** Executes the algorithm. Reading in the file and creating and populating @@ -95,13 +96,16 @@ void LoadParameterFile::exec() // Retrieve the filename from the properties std::string filename = getPropertyValue("Filename"); + // Retrieve the parameter XML string from the properties + std::string parameterXML = getPropertyValue("ParameterXML"); + // Get the input workspace const MatrixWorkspace_sptr localWorkspace = getProperty("Workspace"); - execManually(filename, localWorkspace); + execManually(false, filename, parameterXML, localWorkspace); } -void LoadParameterFile::execManually(std::string filename, Mantid::API::ExperimentInfo_sptr localWorkspace) +void LoadParameterFile::execManually(bool useString, std::string filename, std::string parameterXML, Mantid::API::ExperimentInfo_sptr localWorkspace) { // TODO: Refactor to remove the need for the const cast Instrument_sptr instrument = boost::const_pointer_cast(localWorkspace->getInstrument()->baseInstrument()); From 94586525f2323cff119ce4efcb8f76a03dc14445 Mon Sep 17 00:00:00 2001 From: Karl Palmen Date: Fri, 6 Dec 2013 11:37:10 +0000 Subject: [PATCH 2/8] Use AutoPtr in LoadParameterFile.cpp re #8546 Signed-off-by: Karl Palmen --- .../Framework/DataHandling/src/LoadParameterFile.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp index 6ec3424405a2..760a42894900 100644 --- a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp +++ b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp @@ -35,6 +35,7 @@ This algorithm allows instrument parameters to be specified in a separate file f #include #include #include +#include #include #include #include "MantidGeometry/Instrument/InstrumentDefinitionParser.h" @@ -46,6 +47,7 @@ using Poco::XML::Node; using Poco::XML::NodeList; using Poco::XML::NodeIterator; using Poco::XML::NodeFilter; +using Poco::XML::AutoPtr; using Mantid::Geometry::InstrumentDefinitionParser; @@ -112,7 +114,7 @@ void LoadParameterFile::execManually(bool useString, std::string filename, std:: // Set up the DOM parser and parse xml file DOMParser pParser; - Document* pDoc; + AutoPtr pDoc; try { pDoc = pParser.parse(filename); @@ -127,7 +129,7 @@ void LoadParameterFile::execManually(bool useString, std::string filename, std:: } // Get pointer to root element - Element* pRootElem = pDoc->documentElement(); + AutoPtr pRootElem = pDoc->documentElement(); if ( !pRootElem->hasChildNodes() ) { throw Kernel::Exception::InstrumentDefinitionError("No root element in XML Parameter file", filename); @@ -140,7 +142,6 @@ void LoadParameterFile::execManually(bool useString, std::string filename, std:: // populate parameter map of workspace localWorkspace->populateInstrumentParameters(); - pDoc->release(); } } // namespace DataHandling From 86a24a86f8cabf34c3627daf56017a6210e72451 Mon Sep 17 00:00:00 2001 From: Karl Palmen Date: Fri, 6 Dec 2013 16:10:19 +0000 Subject: [PATCH 3/8] Extend parameterXML code (still inactive) re #8546 Signed-off-by: Karl Palmen --- .../DataHandling/src/LoadParameterFile.cpp | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp index 760a42894900..805a32141161 100644 --- a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp +++ b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp @@ -115,17 +115,35 @@ void LoadParameterFile::execManually(bool useString, std::string filename, std:: // Set up the DOM parser and parse xml file DOMParser pParser; AutoPtr pDoc; - try - { - pDoc = pParser.parse(filename); - } - catch(Poco::Exception& exc) - { - throw Kernel::Exception::FileError(exc.displayText() + ". Unable to parse File:", filename); - } - catch(...) + + if(useString){ + try + { + pDoc = pParser.parseString(parameterXML); + } + catch(Poco::Exception& exc) + { + throw Kernel::Exception::FileError (exc.displayText() + ". Unable to parse parameter XML string","ParameterXML"); + } + catch(...) + { + throw Kernel::Exception::FileError("Unable to parse parameter XML string","ParameterXML"); + } + } + else { - throw Kernel::Exception::FileError("Unable to parse File:" , filename); + try + { + pDoc = pParser.parse(filename); + } + catch(Poco::Exception& exc) + { + throw Kernel::Exception::FileError(exc.displayText() + ". Unable to parse File:", filename); + } + catch(...) + { + throw Kernel::Exception::FileError("Unable to parse File:" , filename); + } } // Get pointer to root element From 4a96bab3ca8dfb21b515c65ae038eea91dba8b4f Mon Sep 17 00:00:00 2001 From: Karl Palmen Date: Mon, 9 Dec 2013 15:39:18 +0000 Subject: [PATCH 4/8] Activate new code and remove unneeded AutoPtr re #8546 Signed-off-by: Karl Palmen --- Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp index 805a32141161..a654ebc6865b 100644 --- a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp +++ b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp @@ -99,12 +99,13 @@ void LoadParameterFile::exec() std::string filename = getPropertyValue("Filename"); // Retrieve the parameter XML string from the properties + const Property * const parameterXMLProperty = getProperty("ParameterXML"); // to check whether it is default std::string parameterXML = getPropertyValue("ParameterXML"); // Get the input workspace const MatrixWorkspace_sptr localWorkspace = getProperty("Workspace"); - execManually(false, filename, parameterXML, localWorkspace); + execManually(!parameterXMLProperty->isDefault(), filename, parameterXML, localWorkspace); } void LoadParameterFile::execManually(bool useString, std::string filename, std::string parameterXML, Mantid::API::ExperimentInfo_sptr localWorkspace) @@ -147,7 +148,7 @@ void LoadParameterFile::execManually(bool useString, std::string filename, std:: } // Get pointer to root element - AutoPtr pRootElem = pDoc->documentElement(); + Element* pRootElem = pDoc->documentElement(); if ( !pRootElem->hasChildNodes() ) { throw Kernel::Exception::InstrumentDefinitionError("No root element in XML Parameter file", filename); From 7fc14c6e7b5db84f696921867a305a667fdd597c Mon Sep 17 00:00:00 2001 From: Karl Palmen Date: Mon, 9 Dec 2013 16:49:45 +0000 Subject: [PATCH 5/8] Check file and string property re #8546 At least one of these two properties must be set. Signed-off-by: Karl Palmen --- Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp index a654ebc6865b..d95efb1e5085 100644 --- a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp +++ b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp @@ -102,6 +102,11 @@ void LoadParameterFile::exec() const Property * const parameterXMLProperty = getProperty("ParameterXML"); // to check whether it is default std::string parameterXML = getPropertyValue("ParameterXML"); + // Check the two properties (at least one must be set) + if( filename.empty() && parameterXMLProperty->isDefault()){ + throw Kernel::Exception::FileError("Either the Filename or ParameterXML property of LoadInstrument most be specified to load an IDF" , filename); + } + // Get the input workspace const MatrixWorkspace_sptr localWorkspace = getProperty("Workspace"); From 254d7579796c2120456b3c06917906f615ca2f64 Mon Sep 17 00:00:00 2001 From: Karl Palmen Date: Tue, 10 Dec 2013 15:22:13 +0000 Subject: [PATCH 6/8] Add unit test to test new property re #8546 Signed-off-by: Karl Palmen --- .../DataHandling/test/LoadParameterFileTest.h | 101 +++++++++++++++--- 1 file changed, 84 insertions(+), 17 deletions(-) diff --git a/Code/Mantid/Framework/DataHandling/test/LoadParameterFileTest.h b/Code/Mantid/Framework/DataHandling/test/LoadParameterFileTest.h index 5c16406ed974..b92b1fdbf314 100644 --- a/Code/Mantid/Framework/DataHandling/test/LoadParameterFileTest.h +++ b/Code/Mantid/Framework/DataHandling/test/LoadParameterFileTest.h @@ -30,24 +30,9 @@ class LoadParameterFileTest : public CxxTest::TestSuite void testExecIDF_for_unit_testing2() // IDF stands for Instrument Definition File { - LoadInstrument loaderIDF2; - - TS_ASSERT_THROWS_NOTHING(loaderIDF2.initialize()); - - //create a workspace with some sample data - wsName = "LoadParameterFileTestIDF2"; - Workspace_sptr ws = WorkspaceFactory::Instance().create("Workspace2D",1,1,1); - Workspace2D_sptr ws2D = boost::dynamic_pointer_cast(ws); - - //put this workspace in the data service - TS_ASSERT_THROWS_NOTHING(AnalysisDataService::Instance().add(wsName, ws2D)); - // Path to test input file assumes Test directory checked out from SVN - loaderIDF2.setPropertyValue("Filename", "IDFs_for_UNIT_TESTING/IDF_for_UNIT_TESTING2.xml"); - //inputFile = loaderIDF2.getPropertyValue("Filename"); - loaderIDF2.setPropertyValue("Workspace", wsName); - TS_ASSERT_THROWS_NOTHING(loaderIDF2.execute()); - TS_ASSERT( loaderIDF2.isExecuted() ); + // Create workspace wsName + load_IDF2(); // load in additional parameters LoadParameterFile loaderPF; @@ -91,6 +76,88 @@ class LoadParameterFileTest : public CxxTest::TestSuite } + void testExec_withIDFString() // Test use of string instead of file + { + + // Create workspace + load_IDF2(); + + // Define parameter XML string + std::string parameterXML = + "" + "" + " " + " " + " " + " " + " " + " " + ""; + + // load in additional parameters + LoadParameterFile loaderPF; + TS_ASSERT_THROWS_NOTHING(loaderPF.initialize()); + loaderPF.setPropertyValue("ParameterXML", parameterXML); + loaderPF.setPropertyValue("Workspace", wsName); + TS_ASSERT_THROWS_NOTHING(loaderPF.execute()); + TS_ASSERT( loaderPF.isExecuted() ); + + // Get back the saved workspace + MatrixWorkspace_sptr output; + TS_ASSERT_THROWS_NOTHING(output = AnalysisDataService::Instance().retrieveWS(wsName)); + + ParameterMap& paramMap = output->instrumentParameters(); + boost::shared_ptr i = output->getInstrument(); + boost::shared_ptr ptrDet = i->getDetector(1008); + TS_ASSERT_EQUALS( ptrDet->getID(), 1008); + TS_ASSERT_EQUALS( ptrDet->getName(), "combined translation6"); + Parameter_sptr param = paramMap.get(&(*ptrDet), "fjols"); + TS_ASSERT_DELTA( param->value(), 20.0, 0.0001); + param = paramMap.get(&(*ptrDet), "nedtur"); + TS_ASSERT_DELTA( param->value(), 77.0, 0.0001); + param = paramMap.get(&(*ptrDet), "fjols-test-paramfile"); + TS_ASSERT_DELTA( param->value(), 52.0, 0.0001); + + std::vector dummy = paramMap.getDouble("nickel-holder", "klovn"); + TS_ASSERT_DELTA( dummy[0], 1.0, 0.0001); + dummy = paramMap.getDouble("nickel-holder", "pos"); + TS_ASSERT_EQUALS (dummy.size(), 0); + dummy = paramMap.getDouble("nickel-holder", "rot"); + TS_ASSERT_EQUALS (dummy.size(), 0); + dummy = paramMap.getDouble("nickel-holder", "taabe"); + TS_ASSERT_DELTA (dummy[0], 200.0, 0.0001); + dummy = paramMap.getDouble("nickel-holder", "mistake"); + TS_ASSERT_EQUALS (dummy.size(), 0); + dummy = paramMap.getDouble("nickel-holder", "fjols-test-paramfile"); + TS_ASSERT_DELTA (dummy[0], 2010.0, 0.0001); + + AnalysisDataService::Instance().remove(wsName); + + } + + void load_IDF2() + { + LoadInstrument loaderIDF2; + + TS_ASSERT_THROWS_NOTHING(loaderIDF2.initialize()); + + //create a workspace with some sample data + wsName = "LoadParameterFileTestIDF2"; + Workspace_sptr ws = WorkspaceFactory::Instance().create("Workspace2D",1,1,1); + Workspace2D_sptr ws2D = boost::dynamic_pointer_cast(ws); + + //put this workspace in the data service + TS_ASSERT_THROWS_NOTHING(AnalysisDataService::Instance().add(wsName, ws2D)); + + // Path to test input file assumes Test directory checked out from git + loaderIDF2.setPropertyValue("Filename", "IDFs_for_UNIT_TESTING/IDF_for_UNIT_TESTING2.xml"); + //inputFile = loaderIDF2.getPropertyValue("Filename"); + loaderIDF2.setPropertyValue("Workspace", wsName); + TS_ASSERT_THROWS_NOTHING(loaderIDF2.execute()); + TS_ASSERT( loaderIDF2.isExecuted() ); + + } + private: std::string inputFile; From f496828dd5bc5792f416635394ba42388b3e6557 Mon Sep 17 00:00:00 2001 From: Karl Palmen Date: Tue, 10 Dec 2013 16:23:08 +0000 Subject: [PATCH 7/8] Add test for absence of file or string re #8546 Also correct the error message. Signed-off-by: Karl Palmen --- .../DataHandling/src/LoadParameterFile.cpp | 2 +- .../DataHandling/test/LoadParameterFileTest.h | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp index d95efb1e5085..439e02a61f94 100644 --- a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp +++ b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp @@ -104,7 +104,7 @@ void LoadParameterFile::exec() // Check the two properties (at least one must be set) if( filename.empty() && parameterXMLProperty->isDefault()){ - throw Kernel::Exception::FileError("Either the Filename or ParameterXML property of LoadInstrument most be specified to load an IDF" , filename); + throw Kernel::Exception::FileError("Either the Filename or ParameterXML property of LoadParameterFile most be specified to load an IDF" , filename); } // Get the input workspace diff --git a/Code/Mantid/Framework/DataHandling/test/LoadParameterFileTest.h b/Code/Mantid/Framework/DataHandling/test/LoadParameterFileTest.h index b92b1fdbf314..e6efa9b134b0 100644 --- a/Code/Mantid/Framework/DataHandling/test/LoadParameterFileTest.h +++ b/Code/Mantid/Framework/DataHandling/test/LoadParameterFileTest.h @@ -135,6 +135,19 @@ class LoadParameterFileTest : public CxxTest::TestSuite } + void test_failure_if_no_file_or_string() + { + // Create workspace + load_IDF2(); + + // Run algorithm without file or string properties set + LoadParameterFile loaderPF; + TS_ASSERT_THROWS_NOTHING(loaderPF.initialize()); + loaderPF.setPropertyValue("Workspace", wsName); + TS_ASSERT_THROWS_NOTHING(loaderPF.execute()); + TS_ASSERT( ! loaderPF.execute() ) + } + void load_IDF2() { LoadInstrument loaderIDF2; From c21d90af98edf77c86f502212e13563edee3ee0b Mon Sep 17 00:00:00 2001 From: Karl Palmen Date: Wed, 11 Dec 2013 11:25:51 +0000 Subject: [PATCH 8/8] Add reference to ticket #8521 in comment re #8546 Signed-off-by: Karl Palmen --- Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp index 439e02a61f94..128ff57d5ba7 100644 --- a/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp +++ b/Code/Mantid/Framework/DataHandling/src/LoadParameterFile.cpp @@ -115,7 +115,7 @@ void LoadParameterFile::exec() void LoadParameterFile::execManually(bool useString, std::string filename, std::string parameterXML, Mantid::API::ExperimentInfo_sptr localWorkspace) { - // TODO: Refactor to remove the need for the const cast + // TODO: Refactor to remove the need for the const cast (ticket #8521) Instrument_sptr instrument = boost::const_pointer_cast(localWorkspace->getInstrument()->baseInstrument()); // Set up the DOM parser and parse xml file