From 5ee366d160901d6b88d40f8e2e68fc233600d91f Mon Sep 17 00:00:00 2001 From: Alex Buts Date: Thu, 24 Apr 2014 17:22:36 +0100 Subject: [PATCH 1/7] refs #9364 Performance test --- .../test/CopyInstrumentParametersTest.h | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/Code/Mantid/Framework/Algorithms/test/CopyInstrumentParametersTest.h b/Code/Mantid/Framework/Algorithms/test/CopyInstrumentParametersTest.h index f010f9fb8bec..98dff9ef48f4 100644 --- a/Code/Mantid/Framework/Algorithms/test/CopyInstrumentParametersTest.h +++ b/Code/Mantid/Framework/Algorithms/test/CopyInstrumentParametersTest.h @@ -191,6 +191,105 @@ class CopyInstrumentParametersTest : public CxxTest::TestSuite CopyInstrumentParameters copyInstParam; +}; + + +class CopyInstrumentParametersTestPerformance : 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 CopyInstrumentParametersTestPerformance *createSuite() { return new CopyInstrumentParametersTestPerformance(); } + static void destroySuite( CopyInstrumentParametersTestPerformance *suite ) { delete suite; } + + + CopyInstrumentParametersTestPerformance(): + m_SourceWSName("SourceWS"),m_TargetWSName("TargWS") + { + size_t n_detectors=44327; + size_t n_Parameters=200; + // Create input workspace with parameterized instrument and put into data store + MatrixWorkspace_sptr ws1 = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(n_detectors+2, 10, true,false,true,"Instr_calibrated"); + AnalysisDataServiceImpl & dataStore = AnalysisDataService::Instance(); + dataStore.add(m_SourceWSName, ws1); + + + Geometry::Instrument_const_sptr instrument = ws1->getInstrument(); + Geometry::ParameterMap *pmap; + pmap = &(ws1->instrumentParameters()); + for(size_t i=0;iaddDouble(instrument.get(),"Param-"+boost::lexical_cast(i),static_cast(i*10)); + } + // calibrate detectors; + for(size_t i=0;igetDetector(1); + Geometry::ComponentHelper::moveComponent(*det, *pmap, V3D(sin(3.1415926*double(i)),cos(3.1415926*double(i/500)),7), Absolute ); + } + + // Create output workspace with another parameterized instrument and put into data store + MatrixWorkspace_sptr ws2 = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(n_detectors, 10, true,false,true,"Instr_base"); + dataStore.add(m_TargetWSName, ws2); + + + copyInstParam.initialize(); + + } + +public: + + void test_copy_performance() + { + // Set properties + TS_ASSERT_THROWS_NOTHING(copyInstParam.setPropertyValue("InputWorkspace", m_SourceWSName )); + TS_ASSERT_THROWS_NOTHING(copyInstParam.setPropertyValue("OutputWorkspace", m_TargetWSName )); + + // Execute Algorithm, should warn but proceed + copyInstParam.setRethrows(true); + TS_ASSERT(copyInstParam.execute()); + TS_ASSERT( copyInstParam.isExecuted() ); + TS_ASSERT(copyInstParam.isInstrumentDifferent()); + + MatrixWorkspace_sptr ws2 =copyInstParam.getProperty("OutputWorkspace"); + auto instr2=ws2->getInstrument(); + + + std::set param_names = instr2->getParameterNames(); + + for(auto it =param_names.begin(); it!=param_names.end();it++) + { + auto name = *it; + double num = boost::lexical_cast(name.substr(6,name.size()-6)); + double val = instr2->getNumberParameter(name)[0]; + TS_ASSERT_DELTA(num,val,1.e-8); + } + + // new detector allocation applied + size_t nDetectors = ws2->getNumberHistograms(); + for(size_t i=0;igetDetector(i); + int id = deto1->getID(); + V3D newPos1 = deto1->getPos(); + TS_ASSERT_EQUALS( id, i+1); + TS_ASSERT_DELTA( newPos1.X() ,sin(3.1415926*double(id)), 0.0001); + TS_ASSERT_DELTA( newPos1.Y() ,cos(3.1415926*double(i/500)), 0.0001); + TS_ASSERT_DELTA( newPos1.Z() , 7, 1.e-6); + + } + + AnalysisDataServiceImpl & dataStore = AnalysisDataService::Instance(); + dataStore.remove(m_SourceWSName); + dataStore.remove(m_TargetWSName); + + } +private: + CopyInstrumentParameters copyInstParam; + const std::string m_SourceWSName,m_TargetWSName; + + }; #endif /*COPYINSTRUMENTPARAMETERSTEST_H_*/ From d7d614ed1027f5ff9e2357f49273018baf2c24af Mon Sep 17 00:00:00 2001 From: Alex Buts Date: Thu, 24 Apr 2014 18:43:45 +0100 Subject: [PATCH 2/7] refs #9364 Working performance test --- .../test/CopyInstrumentParametersTest.h | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/Code/Mantid/Framework/Algorithms/test/CopyInstrumentParametersTest.h b/Code/Mantid/Framework/Algorithms/test/CopyInstrumentParametersTest.h index 98dff9ef48f4..6ba3a32b5df3 100644 --- a/Code/Mantid/Framework/Algorithms/test/CopyInstrumentParametersTest.h +++ b/Code/Mantid/Framework/Algorithms/test/CopyInstrumentParametersTest.h @@ -21,6 +21,7 @@ #include "MantidGeometry/Instrument/ComponentHelper.h" #include + using namespace Mantid::Algorithms; using namespace Mantid::API; using namespace Mantid::Kernel; @@ -209,7 +210,7 @@ class CopyInstrumentParametersTestPerformance : public CxxTest::TestSuite size_t n_detectors=44327; size_t n_Parameters=200; // Create input workspace with parameterized instrument and put into data store - MatrixWorkspace_sptr ws1 = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(n_detectors+2, 10, true,false,true,"Instr_calibrated"); + MatrixWorkspace_sptr ws1 = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(static_cast(n_detectors+2), 10, true,false,true,"Instr_calibrated"); AnalysisDataServiceImpl & dataStore = AnalysisDataService::Instance(); dataStore.add(m_SourceWSName, ws1); @@ -225,12 +226,12 @@ class CopyInstrumentParametersTestPerformance : public CxxTest::TestSuite // calibrate detectors; for(size_t i=0;igetDetector(1); + IComponent_const_sptr det =instrument->getDetector(static_cast(i+1)); Geometry::ComponentHelper::moveComponent(*det, *pmap, V3D(sin(3.1415926*double(i)),cos(3.1415926*double(i/500)),7), Absolute ); } // Create output workspace with another parameterized instrument and put into data store - MatrixWorkspace_sptr ws2 = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(n_detectors, 10, true,false,true,"Instr_base"); + MatrixWorkspace_sptr ws2 = WorkspaceCreationHelper::create2DWorkspaceWithFullInstrument(static_cast(n_detectors), 10, true,false,true,"Instr_base"); dataStore.add(m_TargetWSName, ws2); @@ -242,17 +243,27 @@ class CopyInstrumentParametersTestPerformance : public CxxTest::TestSuite void test_copy_performance() { + // Set properties TS_ASSERT_THROWS_NOTHING(copyInstParam.setPropertyValue("InputWorkspace", m_SourceWSName )); TS_ASSERT_THROWS_NOTHING(copyInstParam.setPropertyValue("OutputWorkspace", m_TargetWSName )); + // Execute Algorithm, should warn but proceed copyInstParam.setRethrows(true); + + clock_t t_start = clock(); TS_ASSERT(copyInstParam.execute()); + clock_t t_end = clock(); + + double seconds=static_cast(t_end-t_start)/static_cast(CLOCKS_PER_SEC); + std::cout<<" Time to copy all parameters is: "<(m_TargetWSName); auto instr2=ws2->getInstrument(); @@ -263,7 +274,7 @@ class CopyInstrumentParametersTestPerformance : public CxxTest::TestSuite auto name = *it; double num = boost::lexical_cast(name.substr(6,name.size()-6)); double val = instr2->getNumberParameter(name)[0]; - TS_ASSERT_DELTA(num,val,1.e-8); + TS_ASSERT_DELTA(num*10,val,1.e-8); } // new detector allocation applied @@ -274,13 +285,12 @@ class CopyInstrumentParametersTestPerformance : public CxxTest::TestSuite int id = deto1->getID(); V3D newPos1 = deto1->getPos(); TS_ASSERT_EQUALS( id, i+1); - TS_ASSERT_DELTA( newPos1.X() ,sin(3.1415926*double(id)), 0.0001); + TS_ASSERT_DELTA( newPos1.X() ,sin(3.1415926*double(i)), 0.0001); TS_ASSERT_DELTA( newPos1.Y() ,cos(3.1415926*double(i/500)), 0.0001); TS_ASSERT_DELTA( newPos1.Z() , 7, 1.e-6); } - AnalysisDataServiceImpl & dataStore = AnalysisDataService::Instance(); dataStore.remove(m_SourceWSName); dataStore.remove(m_TargetWSName); From ae9556712b19e228518a579702259742375ee74d Mon Sep 17 00:00:00 2001 From: Alex Buts Date: Fri, 25 Apr 2014 16:18:07 +0100 Subject: [PATCH 3/7] refs #9364 Add parameter through pointer implemented and deployed within CopyInstrumentParameters. The map is now copy of itself ? (clear way to introduce cow_ptr to map) --- .../src/CopyInstrumentParameters.cpp | 17 +--- .../MantidGeometry/Instrument/ParameterMap.h | 17 ++++ .../Geometry/src/Instrument/ParameterMap.cpp | 92 +++++++++++++++++-- .../Geometry/test/ParameterMapTest.h | 52 ++++++++--- 4 files changed, 142 insertions(+), 36 deletions(-) diff --git a/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp b/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp index ff4be2b61c60..a5f73a2b50f2 100644 --- a/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp +++ b/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp @@ -83,19 +83,6 @@ void CopyInstrumentParameters::exec() Geometry::ParameterMap targMap; - //// vector of all components contained in the target instrument - //std::vector targComponents; - //// flattens instrument definition tree - //inst2->getChildren(targComponents,true); - //// multimap of existing instrument parameters - //std::multimap existingComponents; - //for(size_t i=0;i(targComponents[i].get())) - // continue; - // existingComponents.insert(std::pair(targComponents[i]->getFullName(),targComponents[i].get())); - //} - auto it = givParams.begin(); for(;it!= givParams.end(); it++) { @@ -130,8 +117,8 @@ void CopyInstrumentParameters::exec() targComp = spTargComp->getBaseComponent(); } // merge maps for existing target component - auto param = it->second.get(); - targMap.add(param->type(),targComp,param->name(),param->asString()); + auto param = it->second; //.get(); + targMap.add(targComp, param); } diff --git a/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/ParameterMap.h b/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/ParameterMap.h index 4d5f68c48a92..87c8082c403f 100644 --- a/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/ParameterMap.h +++ b/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/ParameterMap.h @@ -59,6 +59,16 @@ namespace Geometry File change history is stored at: . Code Documentation is available at: */ +#ifndef HAS_UNORDERED_MAP_H + /// Parameter map iterator typedef + typedef std::multimap >::iterator component_map_it; + typedef std::multimap >::const_iterator component_map_cit; +#else + /// Parameter map iterator typedef + typedef std::tr1::unordered_multimap >::iterator component_map_it; + typedef std::tr1::unordered_multimap >::const_iterator component_map_cit; +#endif + class MANTID_GEOMETRY_DLL ParameterMap { public: @@ -148,6 +158,8 @@ namespace Geometry } } } + /// Method for adding a parameter providing shared pointer to it. The class stores share pointer and increment ref count to it + void add(const IComponent* comp,const boost::shared_ptr ¶m); /** @name Helper methods for adding and updating parameter types */ /// Create or adjust "pos" parameter for a component void addPositionCoordinate(const IComponent* comp,const std::string& name, const double value); @@ -285,6 +297,11 @@ namespace Geometry /// Retrieve a parameter by either creating a new one of getting an existing one Parameter_sptr retrieveParameter(bool &created, const std::string & type, const IComponent* comp, const std::string & name); + /// internal function to get position of the parameter in the parameter map + component_map_it getMapPlace(const IComponent* comp,const char *name, const char * type); + ///const version of the internal function to get position of the parameter in the parameter map + component_map_cit getMapPlace(const IComponent* comp,const char *name, const char * type)const; + /// internal parameter map instance pmap m_map; diff --git a/Code/Mantid/Framework/Geometry/src/Instrument/ParameterMap.cpp b/Code/Mantid/Framework/Geometry/src/Instrument/ParameterMap.cpp index 7544be861f05..baa2d7fbbbe3 100644 --- a/Code/Mantid/Framework/Geometry/src/Instrument/ParameterMap.cpp +++ b/Code/Mantid/Framework/Geometry/src/Instrument/ParameterMap.cpp @@ -247,6 +247,30 @@ namespace Mantid } } + /** Method for adding a parameter providing shared pointer to it. + * @param comp :: A pointer to the component that this parameter is attached to + * @param par :: a shared pointer to existing parameter. The ParameterMap stores share pointer and increment ref count to it + */ + void ParameterMap::add(const IComponent* comp,const boost::shared_ptr &par) + { + // can not add null pointer + if(!par)return; + + PARALLEL_CRITICAL(parameter_add) + { + auto existing_par = getMapPlace(comp,par->name().c_str(),""); + if (existing_par != m_map.end()) + { + existing_par->second = par; + } + else + { + m_map.insert(std::make_pair(comp->getComponentID(),par)); + } + } + + } + /** Create or adjust "pos" parameter for a component * Assumed that name either equals "x", "y" or "z" otherwise this * method will not add or modify "pos" parameter @@ -571,11 +595,62 @@ namespace Mantid { Parameter_sptr result; if(!comp) return result; - const bool anytype = (strlen(type) == 0); + PARALLEL_CRITICAL(ParameterMap_get) { - if( !m_map.empty() ) - { + auto itr = getMapPlace(comp,name, type); + if (itr != m_map.end()) + result = itr->second; + } + return result; + } + + /**Return an iterator pointing to a named parameter of a given type. + * @param comp :: Component to which parameter is related + * @param name :: Parameter name + * @param type :: An optional type string. If empty, any type is returned + * @returns The iterator parameter of the given type if it exists or a NULL shared pointer if not + */ + component_map_it ParameterMap::getMapPlace(const IComponent* comp,const char *name, const char * type) + { + pmap_it result = m_map.end(); + if(!comp) return result; + const bool anytype = (strlen(type) == 0); + if( !m_map.empty() ) + { + const ComponentID id = comp->getComponentID(); + pmap_it it_found = m_map.find(id); + if (it_found != m_map.end()) + { + pmap_it itr = m_map.lower_bound(id); + pmap_it itr_end = m_map.upper_bound(id); + for( ; itr != itr_end; ++itr ) + { + Parameter_sptr param = itr->second; + if( boost::iequals(param->nameAsCString(), name) && (anytype || param->type() == type) ) + { + result = itr; + break; + } + } + } + } + return result; + } + + /**Return a const iterator pointing to a named parameter of a given type. + * @param comp :: Component to which parameter is related + * @param name :: Parameter name + * @param type :: An optional type string. If empty, any type is returned + * @returns The iterator parameter of the given type if it exists or a NULL shared pointer if not + */ + component_map_cit ParameterMap::getMapPlace(const IComponent* comp,const char *name, const char * type)const + { + pmap_cit result = m_map.end(); + if(!comp) return result; + const bool anytype = (strlen(type) == 0); + if( !m_map.empty() ) + { const ComponentID id = comp->getComponentID(); pmap_cit it_found = m_map.find(id); if (it_found != m_map.end()) @@ -587,16 +662,17 @@ namespace Mantid Parameter_sptr param = itr->second; if( boost::iequals(param->nameAsCString(), name) && (anytype || param->type() == type) ) { - result = param; + result = itr; break; } } } - } - } + } return result; } + + /** Look for a parameter in the given component by the type of the parameter. * @param comp :: Component to which parameter is related * @param type :: Parameter type @@ -784,7 +860,7 @@ namespace Mantid } } - ///Attempts to retreive a location from the location cache + ///Attempts to retrieve a location from the location cache /// @param comp :: The Component to find the location of /// @param location :: If the location is found it's value will be set here /// @returns true if the location is in the map, otherwise false @@ -810,7 +886,7 @@ namespace Mantid } } - ///Attempts to retreive a rotation from the rotation cache + ///Attempts to retrieve a rotation from the rotation cache /// @param comp :: The Component to find the rotation of /// @param rotation :: If the rotation is found it's value will be set here /// @returns true if the rotation is in the map, otherwise false diff --git a/Code/Mantid/Framework/Geometry/test/ParameterMapTest.h b/Code/Mantid/Framework/Geometry/test/ParameterMapTest.h index c77797d8c161..f9e6f49ef599 100644 --- a/Code/Mantid/Framework/Geometry/test/ParameterMapTest.h +++ b/Code/Mantid/Framework/Geometry/test/ParameterMapTest.h @@ -61,6 +61,7 @@ class ParameterMapTest : public CxxTest::TestSuite ParameterMap pmapA; ParameterMap pmapB; + ParameterMap pmapG; // Empty TS_ASSERT_EQUALS(pmapA,pmapB); @@ -69,36 +70,61 @@ class ParameterMapTest : public CxxTest::TestSuite TS_ASSERT_EQUALS(pmapA,pmapA); // Differs from other TS_ASSERT_DIFFERS(pmapA,pmapB); + TS_ASSERT_DIFFERS(pmapA,pmapG); + + auto par= pmapA.getRecursive(m_testInstrument.get(),name); + pmapG.add(m_testInstrument.get(),par); + TS_ASSERT_EQUALS(pmapA,pmapG); // Same name/value/component pmapB.addDouble(m_testInstrument.get(), name, value); // Now equal TS_ASSERT_EQUALS(pmapA,pmapB); + TS_ASSERT_EQUALS(pmapA,pmapG); - ParameterMap pmapC; + //--- C + ParameterMap pmapC,pmapC1; // Same name/value different component IComponent_sptr comp = m_testInstrument->getChild(0); pmapC.addDouble(comp.get(), name, value); + auto par1=pmapC.getRecursive(comp.get(),name); + pmapC1.add(comp.get(), par1); // Differs from other TS_ASSERT_DIFFERS(pmapA,pmapC); + // Equal + TS_ASSERT_EQUALS(pmapC,pmapC1); + + //--- D // Same name/component different value ParameterMap pmapD; pmapD.addDouble(m_testInstrument.get(), name, value + 1.0); // Differs from other TS_ASSERT_DIFFERS(pmapA,pmapD); + // Adding the same replaces the same par + pmapD.add(m_testInstrument.get(), par1); + // Equal + TS_ASSERT_EQUALS(pmapA,pmapD); + + //--- E // Same value/component different name ParameterMap pmapE; pmapE.addDouble(m_testInstrument.get(), name + "_differ", value); // Differs from other TS_ASSERT_DIFFERS(pmapA,pmapE); + //--- F //Different type ParameterMap pmapF; pmapF.addInt(m_testInstrument.get(), name, 5); // Differs from other TS_ASSERT_DIFFERS(pmapA,pmapF); + // Adding the same replaces the same par regardless of type + pmapF.add(m_testInstrument.get(), par1); + // Equal + TS_ASSERT_EQUALS(pmapA,pmapF); + } void testAdding_A_Parameter_That_Is_Not_Present_Puts_The_Parameter_In() @@ -343,24 +369,24 @@ class ParameterMapTest : public CxxTest::TestSuite void test_copy_from_old_pmap_to_new_pmap_with_new_component(){ - IComponent_sptr oldComp = m_testInstrument->getChild(0); - IComponent_sptr newComp = m_testInstrument->getChild(1); + IComponent_sptr oldComp = m_testInstrument->getChild(0); + IComponent_sptr newComp = m_testInstrument->getChild(1); - ParameterMap oldPMap; - oldPMap.addBool(oldComp.get(), "A", false); - oldPMap.addDouble(oldComp.get(), "B", 1.2); + ParameterMap oldPMap; + oldPMap.addBool(oldComp.get(), "A", false); + oldPMap.addDouble(oldComp.get(), "B", 1.2); - ParameterMap newPMap; + ParameterMap newPMap; - TS_ASSERT_DIFFERS(oldPMap,newPMap); + TS_ASSERT_DIFFERS(oldPMap,newPMap); - newPMap.copyFromParameterMap(oldComp.get(),newComp.get(), &oldPMap); + newPMap.copyFromParameterMap(oldComp.get(),newComp.get(), &oldPMap); - TS_ASSERT_EQUALS(newPMap.contains(newComp.get(), "A", ParameterMap::pBool()), true); - TS_ASSERT_EQUALS(newPMap.contains(newComp.get(), "B", ParameterMap::pDouble()), true); + TS_ASSERT_EQUALS(newPMap.contains(newComp.get(), "A", ParameterMap::pBool()), true); + TS_ASSERT_EQUALS(newPMap.contains(newComp.get(), "B", ParameterMap::pDouble()), true); - Parameter_sptr a = newPMap.get(newComp.get(), "A"); - TS_ASSERT_EQUALS( a->value(), false); + Parameter_sptr a = newPMap.get(newComp.get(), "A"); + TS_ASSERT_EQUALS( a->value(), false); } From a46f4cd242591995c50c36b0c0751cc214940450 Mon Sep 17 00:00:00 2001 From: Alex Buts Date: Fri, 25 Apr 2014 17:19:56 +0100 Subject: [PATCH 4/7] refs #9364 clone method on Parameter and test for it. --- .../inc/MantidGeometry/Instrument/Parameter.h | 5 ++++ .../Geometry/test/ParameterMapTest.h | 27 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/Parameter.h b/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/Parameter.h index 9f0b69f8ffda..bb86e91fb6b5 100644 --- a/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/Parameter.h +++ b/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/Parameter.h @@ -79,6 +79,9 @@ namespace Mantid const std::string& name() const { return m_name; } /// Parameter name const char* nameAsCString() const{ return m_name.c_str(); } + + /// type-independent clone method; + virtual Parameter * clone()const=0; /// Returns the value of the property as a string virtual std::string asString() const{return m_str_value;} @@ -135,6 +138,8 @@ namespace Mantid /// Get the value of the parameter inline const Type& operator()()const{ return m_value; } + Parameter * clone()const + { return new ParameterType(*this);} private: friend class ParameterMap; friend class Parameter; diff --git a/Code/Mantid/Framework/Geometry/test/ParameterMapTest.h b/Code/Mantid/Framework/Geometry/test/ParameterMapTest.h index f9e6f49ef599..269e86e733ce 100644 --- a/Code/Mantid/Framework/Geometry/test/ParameterMapTest.h +++ b/Code/Mantid/Framework/Geometry/test/ParameterMapTest.h @@ -1,9 +1,11 @@ #ifndef PARAMETERMAPTEST_H_ #define PARAMETERMAPTEST_H_ +#include "MantidGeometry/Instrument/Parameter.h" #include "MantidGeometry/Instrument/ParameterMap.h" #include "MantidGeometry/Instrument/Detector.h" #include "MantidTestHelpers/ComponentCreationHelper.h" +#include "MantidKernel/V3D.h" #include #include @@ -127,6 +129,31 @@ class ParameterMapTest : public CxxTest::TestSuite } + void testClone() + { + const double value(5.1); + + ParameterMap pmapA,pmapB; + + pmapA.addDouble(m_testInstrument.get(), "testDouble", value); + pmapA.addV3D(m_testInstrument.get(), "testV3D", Mantid::Kernel::V3D(1,2,3)); + + auto parD= pmapA.getRecursive(m_testInstrument.get(),"testDouble"); + auto parV3= pmapA.getRecursive(m_testInstrument.get(),"testV3D"); + + Mantid::Geometry::Parameter *pParD = parD->clone(); + Mantid::Geometry::Parameter *pParV = parV3->clone(); + + TS_ASSERT_EQUALS(pParD->asString(),parD->asString()) + TS_ASSERT_EQUALS(pParV->asString(),parV3->asString()) + + pmapB.add(m_testInstrument.get(),Parameter_sptr(pParD)); + pmapB.add(m_testInstrument.get(),Parameter_sptr(pParV)); + + TS_ASSERT_EQUALS(pmapA,pmapB); + } + + void testAdding_A_Parameter_That_Is_Not_Present_Puts_The_Parameter_In() { // Add a parameter for the first component of the instrument From 9e500a45ec039458a48a087d9d71a1ac023c0408 Mon Sep 17 00:00:00 2001 From: Alex Buts Date: Fri, 25 Apr 2014 17:35:27 +0100 Subject: [PATCH 5/7] refs #9364 CopyInstrumentParameters clones parameters rather then recreates it through strings --- .../Framework/Algorithms/src/CopyInstrumentParameters.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp b/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp index a5f73a2b50f2..abb063df08a0 100644 --- a/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp +++ b/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp @@ -116,8 +116,10 @@ void CopyInstrumentParameters::exec() } targComp = spTargComp->getBaseComponent(); } - // merge maps for existing target component - auto param = it->second; //.get(); + + // create shared pointer to independent copy of original parameter. Would be easy and nice to have cow_pointer instead of shared_ptr in the parameter map. + auto param = Parameter_sptr(it->second->clone()); + // add new parameter to the maps for existing target component targMap.add(targComp, param); } From a8ac6eed0f2ff83aba782bb866bc8ce979218954 Mon Sep 17 00:00:00 2001 From: Alex Buts Date: Fri, 25 Apr 2014 18:00:42 +0100 Subject: [PATCH 6/7] refs #9364 Avoid copying second parameter map. swap contents of new map with contents of old map instead. --- .../Framework/API/inc/MantidAPI/ExperimentInfo.h | 2 ++ Code/Mantid/Framework/API/src/ExperimentInfo.cpp | 10 ++++++++++ .../Algorithms/src/CopyInstrumentParameters.cpp | 2 +- .../inc/MantidGeometry/Instrument/ParameterMap.h | 6 ++++++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/Code/Mantid/Framework/API/inc/MantidAPI/ExperimentInfo.h b/Code/Mantid/Framework/API/inc/MantidAPI/ExperimentInfo.h index 41e5e65a0a5f..ddf8051b4304 100644 --- a/Code/Mantid/Framework/API/inc/MantidAPI/ExperimentInfo.h +++ b/Code/Mantid/Framework/API/inc/MantidAPI/ExperimentInfo.h @@ -73,6 +73,8 @@ namespace API /// Replaces current parameter map with copy of given map void replaceInstrumentParameters(const Geometry::ParameterMap & pmap); + /// exchange contents of current parameter map with contents of other map) + void swapInstrumentParameters(Geometry::ParameterMap & pmap); /// Cache a lookup of grouped detIDs to member IDs void cacheDetectorGroupings(const det2group_map & mapping); diff --git a/Code/Mantid/Framework/API/src/ExperimentInfo.cpp b/Code/Mantid/Framework/API/src/ExperimentInfo.cpp index e5b056cbe020..8da1f2679ab8 100644 --- a/Code/Mantid/Framework/API/src/ExperimentInfo.cpp +++ b/Code/Mantid/Framework/API/src/ExperimentInfo.cpp @@ -316,6 +316,16 @@ namespace API { this->m_parmap.reset(new ParameterMap(pmap)); } + //--------------------------------------------------------------------------------------- + /** + * exchanges contents of current parameter map with contents of other map) + * @ pmap reference to parameter map which would exchange its contents with current map + */ + void ExperimentInfo::swapInstrumentParameters(Geometry::ParameterMap & pmap) + { + this->m_parmap->swap(pmap); + } + //--------------------------------------------------------------------------------------- /** diff --git a/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp b/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp index abb063df08a0..97473749b4e7 100644 --- a/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp +++ b/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp @@ -125,7 +125,7 @@ void CopyInstrumentParameters::exec() } // changed parameters - m_receivingWorkspace->replaceInstrumentParameters( targMap ); + m_receivingWorkspace->swapInstrumentParameters(targMap); } else diff --git a/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/ParameterMap.h b/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/ParameterMap.h index 87c8082c403f..d638377223be 100644 --- a/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/ParameterMap.h +++ b/Code/Mantid/Framework/Geometry/inc/MantidGeometry/Instrument/ParameterMap.h @@ -120,6 +120,12 @@ namespace Geometry m_map.clear(); clearPositionSensitiveCaches(); } + /// method swaps two parameter maps contents each other. All caches contents is nullified (TO DO: it can be efficiently swapped too) + void swap(ParameterMap &other) + { + m_map.swap(other.m_map); + clearPositionSensitiveCaches(); + } /// Clear any parameters with the given name void clearParametersByName(const std::string & name); From 275e70994ccf8c7c0c58a8db6737339c9f952352 Mon Sep 17 00:00:00 2001 From: Alex Buts Date: Fri, 25 Apr 2014 18:08:41 +0100 Subject: [PATCH 7/7] refs #9364 Comments bla bla --- .../Algorithms/src/CopyInstrumentParameters.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp b/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp index 97473749b4e7..049166b41377 100644 --- a/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp +++ b/Code/Mantid/Framework/Algorithms/src/CopyInstrumentParameters.cpp @@ -1,11 +1,12 @@ /*WIKI* -Transfer an instrument from a giving workspace to a receiving workspace for the same instrument. +Transfer an instrument parameters from a giving workspace to a receiving workspace. -The instrument in of the receiving workspace is replaced by a copy of the instrument in the giving workspace -and so gains any manipulations such as calibration done to the instrument in the giving workspace. -The two workspaces can have spectra allocated to their detectors differently. +The instrument parameters in the receiving workspace are REPLACED (despite you can assume from the name of the algorithm) +by a copy of the instrument parameters in the giving workspace +so gaining any manipulations such as calibration done to the instrument in the giving workspace. +Does not work on workspaces with grouped detectors if some of the detectors were calibrated. *WIKI*/ //----------------------------------------------------------------------