Skip to content

Commit

Permalink
Improve the ParameterMap recursive searching code.
Browse files Browse the repository at this point in the history
Avoids instantiation of separate shared pointer & custom deleter.
Refs #8057
  • Loading branch information
martyngigg committed Oct 3, 2013
1 parent c8a6731 commit 061b18f
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ namespace Geometry
inline void clear()
{
m_map.clear();
clearCache();
clearPositionSensitiveCaches();
}
/// Clear any parameters with the given name
void clearParametersByName(const std::string & name);
Expand All @@ -118,10 +118,6 @@ namespace Geometry
void add(const std::string& type,const IComponent* comp,const std::string& name,
const std::string& value);

// /// Method for adding a parameter providing its value as a char string
// void add(const std::string& type,const IComponent* comp,const std::string& name,
// const char* value) {add( type, comp, name, std::string(value));}

/**
* Method for adding a parameter providing its value of a particular type
* @tparam T The concrete type
Expand All @@ -141,7 +137,6 @@ namespace Geometry
ParameterType<T> *paramT = dynamic_cast<ParameterType<T> *>(param.get());
if (!paramT)
{
reportError("Error in adding parameter: incompatible types");
throw std::runtime_error("Error in adding parameter: incompatible types");
}
paramT->setValue(value);
Expand Down Expand Up @@ -178,21 +173,15 @@ namespace Geometry
void addQuat(const IComponent* comp,const std::string& name, const Kernel::Quat& value);
//@}

/// Does the named parameter exist for the given component for any type.
bool contains(const IComponent* comp, const char* name) const;
/// Does the named parameter exist for the given component and type
bool contains(const IComponent* comp, const std::string & name, const std::string & type = "") const;
/// Does the given parameter & component combination exist
bool contains(const IComponent* comp, const Parameter & parameter) const;
/// Get a parameter with a given name
boost::shared_ptr<Parameter> get(const IComponent* comp, const char * name) const;
/// Get a parameter with a given name and optional type
boost::shared_ptr<Parameter> get(const IComponent* comp,const std::string& name,
const std::string & type = "")const;
/// Finds the parameter in the map via the parameter type.
boost::shared_ptr<Parameter> getByType(const IComponent* comp, const std::string& type) const;
/// Use get() recursively to see if can find param in all parents of comp.
boost::shared_ptr<Parameter> getRecursive(const IComponent* comp, const char * name) const;
/// Use get() recursively to see if can find param in all parents of comp and given type
boost::shared_ptr<Parameter> getRecursive(const IComponent* comp,const std::string& name,
const std::string & type = "")const;
Expand Down Expand Up @@ -256,8 +245,8 @@ namespace Geometry
/// Returns a string with all component names, parameter names and values
std::string asString()const;

///Clears the location and roatation caches
void clearCache();
///Clears the location, rotation & bounding box caches
void clearPositionSensitiveCaches();
///Sets a cached location on the location cache
void setCachedLocation(const IComponent* comp, const Kernel::V3D& location) const;
///Attempts to retreive a location from the location cache
Expand All @@ -270,19 +259,15 @@ namespace Geometry
void setCachedBoundingBox(const IComponent *comp, const BoundingBox & box) const;
///Attempts to retrieve a bounding box from the cache
bool getCachedBoundingBox(const IComponent *comp, BoundingBox & box) const;

/// Persist a representation of the Parameter map to the open Nexus file
void saveNexus(::NeXus::File * file, const std::string & group) const;
// void loadNexus(::NeXus::File * file, const std::string & group, Instrument_const_sptr instr);


private:
///Assignment operator
ParameterMap& operator=(ParameterMap * rhs);
/// 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,
Parameter_sptr retrieveParameter(bool &created, const std::string & type, const IComponent* comp,
const std::string & name);
/// report an error
void reportError(const std::string& str);

/// internal parameter map instance
pmap m_map;
Expand All @@ -292,6 +277,7 @@ namespace Geometry
mutable Kernel::Cache<const ComponentID, Kernel::Quat > m_cacheRotMap;
///internal cache map for cached bounding boxes
mutable Kernel::Cache<const ComponentID,BoundingBox> m_boundingBoxMap;

/// Static reference to the logger class
static Kernel::Logger& g_log;
};
Expand Down
20 changes: 9 additions & 11 deletions Code/Mantid/Framework/Geometry/src/Instrument/ParameterMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -593,19 +593,17 @@ namespace Mantid
Parameter_sptr ParameterMap::getRecursive(const IComponent* comp,const std::string& name,
const std::string & type) const
{
if( m_map.empty() ) return Parameter_sptr();
boost::shared_ptr<const IComponent> compInFocus(comp,NoDeleting());
while( compInFocus != NULL )
Parameter_sptr result = this->get(comp->getComponentID(), name, type);
if(result) return result;

auto parent = comp->getParent();
while(parent)
{
Parameter_sptr param = get(compInFocus.get(), name, type);
if (param)
{
return param;
}
compInFocus = compInFocus->getParent();
result = get(parent->getComponentID(), name, type);
if(result) return result;
parent = parent->getParent();
}
//Nothing was found!
return Parameter_sptr();
return result;
}

/**
Expand Down
24 changes: 16 additions & 8 deletions Code/Mantid/Framework/Geometry/test/ParameterMapTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,19 +181,27 @@ class ParameterMapTest : public CxxTest::TestSuite

void testRecursive_Parameter_Search_Moves_Up_The_Instrument_Tree()
{
// Attach a parameter to the instrument
const std::string topLevel("TopLevelParameter");
const int value(2);
// Attach 2 parameters to the instrument
const std::string topLevel1("top1"), topLevel2("top2");
const int value1(2), value2(3);
ParameterMap pmap;
pmap.addInt(m_testInstrument.get(), topLevel, value);
pmap.addInt(m_testInstrument.get(), topLevel1, value1);
pmap.addInt(m_testInstrument.get(), topLevel2, value2);
//Ask for the parameter on a child
IComponent_sptr comp = m_testInstrument->getChild(0);
// Non-recursive should not find the parameter
Parameter_sptr fetched = pmap.get(comp.get(), topLevel);
Parameter_sptr fetched = pmap.get(comp.get(), topLevel1);
TS_ASSERT_EQUALS(fetched, Parameter_sptr());
fetched = pmap.getRecursive(comp.get(), topLevel);

fetched = pmap.getRecursive(comp.get(), topLevel1);
TS_ASSERT(fetched);
TS_ASSERT_EQUALS(fetched->value<int>(), value1);

// Check that the correct parameter name is found even after a first call that
// would be cache the previous one
fetched = pmap.getRecursive(comp.get(), topLevel2);
TS_ASSERT(fetched);
TS_ASSERT_EQUALS(fetched->value<int>(), value);
TS_ASSERT_EQUALS(fetched->value<int>(), value2);
}

void testClearByName_Only_Removes_Named_Parameter()
Expand Down Expand Up @@ -360,7 +368,7 @@ class ParameterMapTestPerformance : public CxxTest::TestSuite
m_testInst->add(subbank1);
m_testInst->add(topbank);

// Add a parameter at the top level
// Add a double parameter at the top level
m_pmap.addDouble(m_testInst->getComponentID(), "instlevel",10.0);
// and at leaf level
m_pmap.addDouble(m_leaf->getComponentID(), "leaflevel",11.0);
Expand Down

0 comments on commit 061b18f

Please sign in to comment.