Skip to content

Commit

Permalink
Merge pull request #44 from mantidproject/bugfix/10375_parameter_map_…
Browse files Browse the repository at this point in the history 
…race_condition

Looks good and everything appears to work. I couldn't find where m_mapAccess was defined though.
  • Loading branch information
@peterfpeterson
peterfpeterson committed Oct 16, 2014
2 parents 9955525 + 7cfbfa6 commit 82f9294
Showing 1 changed file with 46 additions and 47 deletions.
93 changes: 46 additions & 47 deletions Code/Mantid/Framework/Geometry/src/Instrument/ParameterMap.cpp
View file
Expand Up @@ -131,10 +131,10 @@ namespace Mantid
* @return true if the objects are considered not equal, false otherwise
*/
bool ParameterMap::operator!=(const ParameterMap & rhs) const
{
return !(this->operator==(rhs));
{
return !(this->operator==(rhs));
}

/**
* Compares the values in this object with that given for equality
* The order of the values in the map is not important.
Expand All @@ -144,10 +144,10 @@ namespace Mantid
bool ParameterMap::operator==(const ParameterMap & rhs) const
{
if(this == &rhs) return true; // True for the same object

// Quick size check
if(this->size() != rhs.size()) return false;

// The map is unordered and the key is only valid at runtime. The
// asString method turns the ComponentIDs to full-qualified name identifiers
// so we will use the same approach to compare them
Expand Down Expand Up @@ -175,7 +175,7 @@ namespace Mantid
return true;
}


/**
* Output information that helps understanding the mismatch between two parameter maps.
* To loop through the difference between two very large parameter map can take time, in which
Expand All @@ -187,15 +187,15 @@ namespace Mantid
const std::string ParameterMap::diff(const ParameterMap & rhs, const bool & firstDiffOnly) const
{
if(this == &rhs) return std::string(""); // True for the same object

// Quick size check
if(this->size() != rhs.size())
if(this->size() != rhs.size())
{
return std::string("Number of parameters does not match: ") +
return std::string("Number of parameters does not match: ") +
boost::lexical_cast<std::string>(this->size()) + " not equal to " +
boost::lexical_cast<std::string>(rhs.size());
}

// Run this same loops as in operator==
// The map is unordered and the key is only valid at runtime. The
// asString method turns the ComponentIDs to full-qualified name identifiers
Expand All @@ -221,7 +221,7 @@ namespace Mantid
}
}

if(!match)
if(!match)
{
// output some information that helps with understanding the mismatch
strOutput << "Parameter mismatch LHS=RHS for LHS parameter in component with name: " << fullName
Expand Down Expand Up @@ -306,15 +306,15 @@ namespace Mantid
if( name == pos() || name == rot() ) clearPositionSensitiveCaches();
}
}

/**
* Add a value into the map
* @param type :: A string denoting the type, e.g. double, string, fitting
* @param comp :: A pointer to the component that this parameter is attached to
* @param name :: The name of the parameter
* @param value :: The parameter's value
*/
void ParameterMap::add(const std::string& type,const IComponent* comp,const std::string& name,
void ParameterMap::add(const std::string& type,const IComponent* comp,const std::string& name,
const std::string& value)
{
auto param = ParameterFactory::create(type,name);
Expand All @@ -331,7 +331,7 @@ namespace Mantid
// can not add null pointer
if(!par)return;

PARALLEL_CRITICAL(parameter_add)
PARALLEL_CRITICAL(m_mapAccess)
{
auto existing_par = positionOf(comp,par->name().c_str(),"");
// As this is only an add method it should really throw if it already exists.
Expand All @@ -350,13 +350,13 @@ namespace Mantid
}

/** Create or adjust "pos" parameter for a component
* Assumed that name either equals "x", "y" or "z" otherwise this
* Assumed that name either equals "x", "y" or "z" otherwise this
* method will not add or modify "pos" parameter
* @param comp :: Component
* @param name :: name of the parameter
* @param value :: value
*/
void ParameterMap::addPositionCoordinate(const IComponent* comp, const std::string& name,
void ParameterMap::addPositionCoordinate(const IComponent* comp, const std::string& name,
const double value)
{
Parameter_sptr param = get(comp,pos());
Expand Down Expand Up @@ -393,7 +393,7 @@ namespace Mantid
}

/** Create or adjust "rot" parameter for a component
* Assumed that name either equals "rotx", "roty" or "rotz" otherwise this
* Assumed that name either equals "rotx", "roty" or "rotz" otherwise this
* method will not add/modify "rot" parameter
* @param comp :: Component
* @param name :: Parameter name
Expand All @@ -420,7 +420,7 @@ namespace Mantid
rotZ = paramRotZ->value<double>();
else
rotZ = 0.0;


// adjust rotation
Quat quat;
Expand Down Expand Up @@ -452,7 +452,7 @@ namespace Mantid
addQuat(comp, rot(), quat);
}

/**
/**
* Adds a double value to the parameter map.
* @param comp :: Component to which the new parameter is related
* @param name :: Name for the new parameter
Expand All @@ -474,7 +474,7 @@ namespace Mantid
add(pDouble(),comp,name,value);
}

/**
/**
* Adds an int value to the parameter map.
* @param comp :: Component to which the new parameter is related
* @param name :: Name for the new parameter
Expand All @@ -496,7 +496,7 @@ namespace Mantid
add(pInt(),comp,name,value);
}

/**
/**
* Adds a bool value to the parameter map.
* @param comp :: Component to which the new parameter is related
* @param name :: Name for the new parameter
Expand All @@ -517,7 +517,7 @@ namespace Mantid
add(pBool(),comp,name,value);
}

/**
/**
* Adds a std::string value to the parameter map.
* @param comp :: Component to which the new parameter is related
* @param name :: Name for the new parameter
Expand All @@ -528,7 +528,7 @@ namespace Mantid
add<std::string>(pString(),comp,name,value);
}

/**
/**
* Adds a V3D value to the parameter map.
* @param comp :: Component to which the new parameter is related
* @param name :: Name for the new parameter
Expand All @@ -552,7 +552,7 @@ namespace Mantid
clearPositionSensitiveCaches();
}

/**
/**
* Adds a Quat value to the parameter map.
* @param comp :: Component to which the new parameter is related
* @param name :: Name for the new parameter
Expand Down Expand Up @@ -625,7 +625,7 @@ namespace Mantid
}
return false;
}
else
else
return false;
}

Expand Down Expand Up @@ -654,16 +654,16 @@ namespace Mantid
Parameter_sptr result;
if(!comp) return result;

PARALLEL_CRITICAL(ParameterMap_get)
PARALLEL_CRITICAL(m_mapAccess)
{
auto itr = positionOf(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.

/**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
Expand Down Expand Up @@ -692,11 +692,11 @@ namespace Mantid
}
}
}
}
}
return result;
}

/**Return a const iterator pointing to a named parameter of a given type.
/**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
Expand Down Expand Up @@ -725,7 +725,7 @@ namespace Mantid
}
}
}
}
}
return result;
}

Expand All @@ -739,7 +739,7 @@ namespace Mantid
Parameter_sptr ParameterMap::getByType(const IComponent* comp, const std::string& type) const
{
Parameter_sptr result = Parameter_sptr();
PARALLEL_CRITICAL(ParameterMap_get)
PARALLEL_CRITICAL(m_mapAccess)
{
if( !m_map.empty() )
{
Expand All @@ -763,7 +763,7 @@ namespace Mantid
} // found->firdst
} // it_found != m_map.end()
} //!m_map.empty()
} // PARALLEL_CRITICAL(ParameterMap_get)
} // PARALLEL_CRITICAL(m_map_access)
return result;
}

Expand All @@ -789,29 +789,29 @@ namespace Mantid
}


/**
/**
* Find a parameter by name, recursively going up the component tree
* to higher parents.
* @param comp :: The component to start the search with
* @param name :: Parameter name
* @param type :: An optional type string
* @returns the first matching parameter.
*/
Parameter_sptr ParameterMap::getRecursive(const IComponent* comp,const std::string& name,
Parameter_sptr ParameterMap::getRecursive(const IComponent* comp,const std::string& name,
const std::string & type) const
{
return getRecursive(comp, name.c_str(), type.c_str());
}

/**
/**
* Find a parameter by name, recursively going up the component tree
* to higher parents.
* @param comp :: The component to start the search with
* @param name :: Parameter name
* @param type :: An optional type string
* @returns the first matching parameter.
*/
Parameter_sptr ParameterMap::getRecursive(const IComponent* comp, const char* name,
Parameter_sptr ParameterMap::getRecursive(const IComponent* comp, const char* name,
const char * type) const
{
Parameter_sptr result = this->get(comp->getComponentID(), name, type);
Expand All @@ -827,7 +827,7 @@ namespace Mantid
return result;
}

/**
/**
* Return the value of a parameter as a string
* @param comp :: Component to which parameter is related
* @param name :: Parameter name
Expand All @@ -849,7 +849,7 @@ namespace Mantid
return param->asString();
}

/**
/**
* Returns a set with all the parameter names for the given component
* @param comp :: A pointer to the component of interest
* @returns A set of names of parameters for the given component
Expand All @@ -872,8 +872,8 @@ namespace Mantid
}

return paramNames;
}
}

/**
* Return a string representation of the parameter map. The format is either:
* |detID:id-value;param-type;param-name;param-value| for a detector or
Expand Down Expand Up @@ -913,10 +913,10 @@ namespace Mantid
m_cacheRotMap.clear();
m_boundingBoxMap.clear();
}

///Sets a cached location on the location cache
/// @param comp :: The Component to set the location of
/// @param location :: The location
/// @param location :: The location
void ParameterMap::setCachedLocation(const IComponent* comp, const V3D& location) const
{
// Call to setCachedLocation is a write so not thread-safe
Expand All @@ -942,7 +942,7 @@ namespace Mantid

///Sets a cached rotation on the rotation cache
/// @param comp :: The Component to set the rotation of
/// @param rotation :: The rotation as a quaternion
/// @param rotation :: The rotation as a quaternion
void ParameterMap::setCachedRotation(const IComponent* comp, const Quat& rotation) const
{
// Call to setCachedRotation is a write so not thread-safe
Expand Down Expand Up @@ -995,7 +995,7 @@ namespace Mantid
* @param oldPMap :: Old map corresponding to the Old component
*/
void ParameterMap::copyFromParameterMap(const IComponent* oldComp,
const IComponent* newComp, const ParameterMap *oldPMap)
const IComponent* newComp, const ParameterMap *oldPMap)
{

std::set<std::string> oldParameterNames = oldPMap->names(oldComp);
Expand All @@ -1007,7 +1007,7 @@ namespace Mantid
m_map.insert(std::make_pair(newComp->getComponentID(),thisParameter));
}
}

//--------------------------------------------------------------------------------------------
/** Save the object to an open NeXus file.
* @param file :: open NeXus file
Expand All @@ -1028,4 +1028,3 @@ namespace Mantid

} // Namespace Geometry
} // Namespace Mantid

0 comments on commit 82f9294

Please sign in to comment.