Skip to content

Commit

Permalink
Refs #4249.IFunctionMW now holds weak_ptr<Workspace> not a shared_ptr
Browse files Browse the repository at this point in the history 
This ensures that it does not keep a workspace alive unnecessarily, i.e
in the FitBrowser where a function is long lived.
  • Loading branch information
@martyngigg
martyngigg committed Dec 2, 2011
1 parent e5ebc6d commit 0d80c20
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 90 deletions.
15 changes: 12 additions & 3 deletions Code/Mantid/Framework/API/inc/MantidAPI/IFunctionMW.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include <string>
#include <vector>

#include <boost/weak_ptr.hpp>

namespace Mantid
{

Expand Down Expand Up @@ -148,7 +150,11 @@ class MANTID_API_DLL IFunctionMW: public virtual IFitFunction
/// Set the workspace
virtual void setMatrixWorkspace(boost::shared_ptr<const API::MatrixWorkspace> workspace, size_t wi, double startX = 0.0, double endX = 0.0);
/// Get the workspace
virtual boost::shared_ptr<const API::MatrixWorkspace> getMatrixWorkspace()const{return m_workspace;}
virtual boost::shared_ptr<const API::MatrixWorkspace> getMatrixWorkspace() const
{
if(m_workspace.expired()) return boost::shared_ptr<const API::MatrixWorkspace>();
else return m_workspace.lock();
}
/// Get workspace index
virtual size_t getWorkspaceIndex()const{return m_workspaceIndex;}

Expand Down Expand Up @@ -184,8 +190,6 @@ class MANTID_API_DLL IFunctionMW: public virtual IFitFunction
/// Calculate numerical derivatives
void calNumericalDeriv(Jacobian* out, const double* xValues, const size_t& nData);

/// Shared pointer to the workspace
boost::shared_ptr<const API::MatrixWorkspace> m_workspace;
/// Spectrum index
size_t m_workspaceIndex;
/// Lower bin index
Expand All @@ -210,6 +214,11 @@ class MANTID_API_DLL IFunctionMW: public virtual IFitFunction
/// Making a friend
friend class CurveFitting::Fit;

private:
/// Weak pointer to the workspace. Use the getMatrixWorkspace function above
boost::weak_ptr<const API::MatrixWorkspace> m_workspace;


};

} // namespace API
Expand Down
52 changes: 25 additions & 27 deletions Code/Mantid/Framework/API/src/IFunctionMW.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,12 @@ namespace API

void IFunctionMW::setWorkspace(boost::shared_ptr<const Workspace> ws,bool)
{
m_workspace = boost::dynamic_pointer_cast<const MatrixWorkspace>(ws);
if (!m_workspace)
MatrixWorkspace_const_sptr matrix = boost::dynamic_pointer_cast<const MatrixWorkspace>(ws);
if (matrix)
{
m_workspace = boost::weak_ptr<const MatrixWorkspace>(matrix);
}
else
{
throw std::invalid_argument("Workspace has a wrong type (not a MatrixWorkspace)");
}
Expand All @@ -48,9 +52,9 @@ namespace API
*/
void IFunctionMW::setSlicing(const std::string& slicing)
{
if (!m_workspace)
MatrixWorkspace_const_sptr mws = this->getMatrixWorkspace();
if (!mws)
{// unset workspace
m_workspace.reset();
m_workspaceIndex = 0;
m_xMinIndex = 0;
m_xMaxIndex = 0;
Expand All @@ -63,12 +67,6 @@ namespace API

try
{
MatrixWorkspace_const_sptr mws = m_workspace;
if (!mws)
{
throw std::invalid_argument("Workspace has a wrong type (not a MatrixWorkspace)");
}

size_t index = mws->getNumberHistograms();
double startX = 0;
double endX = 0;
Expand Down Expand Up @@ -127,7 +125,7 @@ namespace API
/// Get the workspace
boost::shared_ptr<const API::Workspace> IFunctionMW::getWorkspace()const
{
return m_workspace;
return getMatrixWorkspace();
}

/// Returns the size of the fitted data (number of double values returned by the function)
Expand Down Expand Up @@ -194,21 +192,21 @@ void IFunctionMW::setMatrixWorkspace(boost::shared_ptr<const API::MatrixWorkspac
{
m_workspaceIndex = wi;

m_workspace = workspace;
m_workspace = boost::weak_ptr<const MatrixWorkspace>(workspace);
if (!workspace) return; // unset the workspace

size_t n = m_workspace->blocksize(); // length of each Y vector
size_t n = workspace->blocksize(); // length of each Y vector
size_t xMin = 0;
size_t xMax = 0;

if (wi >= m_workspace->getNumberHistograms())
if (wi >= workspace->getNumberHistograms())
{
throw std::range_error("Workspace index out of range");
}

const MantidVec& x = m_workspace->readX(wi);
const MantidVec& y = m_workspace->readY(wi);
const MantidVec& e = m_workspace->readE(wi);
const MantidVec& x = workspace->readX(wi);
const MantidVec& y = workspace->readY(wi);
const MantidVec& e = workspace->readE(wi);
bool isHist = x.size() > y.size();

if (startX >= endX)
Expand Down Expand Up @@ -277,9 +275,9 @@ void IFunctionMW::setMatrixWorkspace(boost::shared_ptr<const API::MatrixWorkspac
if ( negativeError )
g_log.warning() << "Negative error values found! These are set to absolute value\n";

if (m_workspace->hasMaskedBins(wi))
if (workspace->hasMaskedBins(wi))
{
const MatrixWorkspace::MaskList& mlist = m_workspace->maskedBins(wi);
const MatrixWorkspace::MaskList& mlist = workspace->maskedBins(wi);
MatrixWorkspace::MaskList::const_iterator it = mlist.begin();
for(;it!=mlist.end();++it)
{
Expand All @@ -292,21 +290,21 @@ void IFunctionMW::setMatrixWorkspace(boost::shared_ptr<const API::MatrixWorkspac

// check if parameter are specified in instrument definition file

const Geometry::ParameterMap& paramMap = m_workspace->instrumentParameters();
const Geometry::ParameterMap& paramMap = workspace->instrumentParameters();


Geometry::IDetector_const_sptr det;
size_t numDetectors = m_workspace->getSpectrum(wi)->getDetectorIDs().size() ;
size_t numDetectors = workspace->getSpectrum(wi)->getDetectorIDs().size() ;
if (numDetectors > 1)
{
// If several detectors are on this workspace index, just use the ID of the first detector
// Note JZ oct 2011 - I'm not sure why the code uses the first detector and not the group. Ask Roman.
Instrument_const_sptr inst = m_workspace->getInstrument();
det = inst->getDetector( *m_workspace->getSpectrum(wi)->getDetectorIDs().begin() );
Instrument_const_sptr inst = workspace->getInstrument();
det = inst->getDetector( *workspace->getSpectrum(wi)->getDetectorIDs().begin() );
}
else
// Get the detector (single) at this workspace index
det = m_workspace->getDetector(wi);;
det = workspace->getDetector(wi);;

for (size_t i = 0; i < nParams(); i++)
{
Expand Down Expand Up @@ -352,7 +350,7 @@ void IFunctionMW::setMatrixWorkspace(boost::shared_ptr<const API::MatrixWorkspac

// if unit specified convert centre value to unit required by formula or look-up-table
if (centreUnit)
centreValue = convertValue(centreValue, centreUnit, m_workspace, wi);
centreValue = convertValue(centreValue, centreUnit, workspace, wi);

double paramValue = fitParam.getValue(centreValue);

Expand All @@ -362,7 +360,7 @@ void IFunctionMW::setMatrixWorkspace(boost::shared_ptr<const API::MatrixWorkspac
{
// so from look up table
Kernel::Unit_sptr resultUnit = fitParam.getLookUpTable().getYUnit(); // from table
paramValue /= convertValue(1.0, resultUnit, m_workspace, wi);
paramValue /= convertValue(1.0, resultUnit, workspace, wi);
}
else
{
Expand All @@ -382,7 +380,7 @@ void IFunctionMW::setMatrixWorkspace(boost::shared_ptr<const API::MatrixWorkspac
std::stringstream readDouble;
Kernel::Unit_sptr unt = Kernel::UnitFactory::Instance().create(allUnitStr[iUnit]);
readDouble << 1.0 /
convertValue(1.0, unt, m_workspace, wi);
convertValue(1.0, unt, workspace, wi);
resultUnitStr.replace(found, len, readDouble.str());
}
} // end for
Expand Down
8 changes: 5 additions & 3 deletions Code/Mantid/Framework/CurveFitting/src/IkedaCarpenterPV.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,15 @@ void IkedaCarpenterPV::calWavelengthAtEachDataPoint(const double* xValues, const
// note if a version of convertValue was added which allows a double* as first argument
// then could avoid copying above plus only have to resize m_wavelength when
// its size smaller than nData
if ( m_workspace != 0 )
API::MatrixWorkspace_const_sptr mws = getMatrixWorkspace();
if ( mws )
{
Instrument_const_sptr instrument = m_workspace->getInstrument();
API::MatrixWorkspace_const_sptr mws = getMatrixWorkspace();
Instrument_const_sptr instrument = mws->getInstrument();
Geometry::IObjComponent_const_sptr sample = instrument->getSample();
if (sample != NULL)
{
convertValue(m_waveLength, wavelength, m_workspace, m_workspaceIndex);
convertValue(m_waveLength, wavelength, mws, m_workspaceIndex);
}
else
{
Expand Down
9 changes: 6 additions & 3 deletions Code/Mantid/Framework/CurveFitting/test/CompositeFunctionTest.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ class CurveFittingGauss: public IPeakFunction, public ITestFunction

void testInit(Mantid::DataObjects::Workspace2D_const_sptr ws,int spec,int xMin,int xMax)
{
TS_ASSERT_EQUALS(ws.get(),m_workspace.get());
MatrixWorkspace_const_sptr mws = getMatrixWorkspace();
TS_ASSERT_EQUALS(ws.get(),mws.get());
TS_ASSERT_EQUALS(spec,m_workspaceIndex);
TS_ASSERT_EQUALS(xMin,m_xMinIndex);
TS_ASSERT_EQUALS(xMax,m_xMaxIndex);
Expand Down Expand Up @@ -136,7 +137,8 @@ class CurveFittingLinear: public ParamFunction, public IFunctionMW, public ITest

void testInit(Mantid::DataObjects::Workspace2D_const_sptr ws,int spec,int xMin,int xMax)
{
TS_ASSERT_EQUALS(ws.get(),m_workspace.get());
MatrixWorkspace_const_sptr mws = getMatrixWorkspace();
TS_ASSERT_EQUALS(ws.get(), mws.get());
TS_ASSERT_EQUALS(spec,m_workspaceIndex);
TS_ASSERT_EQUALS(xMin,m_xMinIndex);
TS_ASSERT_EQUALS(xMax,m_xMaxIndex);
Expand All @@ -149,7 +151,8 @@ class CompFunction : public CompositeFunctionMW

void testInit(Mantid::DataObjects::Workspace2D_const_sptr ws,int spec,int xMin,int xMax)
{
TS_ASSERT_EQUALS(ws.get(),m_workspace.get());
MatrixWorkspace_const_sptr mws = getMatrixWorkspace();
TS_ASSERT_EQUALS(ws.get(), mws.get());
TS_ASSERT_EQUALS(spec,m_workspaceIndex);
TS_ASSERT_EQUALS(xMin,m_xMinIndex);
TS_ASSERT_EQUALS(xMax,m_xMaxIndex);
Expand Down
3 changes: 2 additions & 1 deletion Code/Mantid/Framework/CurveFitting/test/FunctionTest.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ class FunctionTestGauss: public IPeakFunction

void testInit(Mantid::DataObjects::Workspace2D_const_sptr ws,int spec,int xMin,int xMax)
{
TS_ASSERT_EQUALS(ws.get(),m_workspace.get());
MatrixWorkspace_const_sptr mws = getMatrixWorkspace();
TS_ASSERT_EQUALS(ws.get(), mws.get());
TS_ASSERT_EQUALS(spec,m_workspaceIndex);
TS_ASSERT_EQUALS(xMin,m_xMinIndex);
TS_ASSERT_EQUALS(xMax,m_xMaxIndex);
Expand Down
2 changes: 0 additions & 2 deletions Code/Mantid/MantidQt/MantidWidgets/inc/MantidQtMantidWidgets/FitPropertyBrowser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,6 @@ private slots:
void doubleChanged(QtProperty* prop);
void stringChanged(QtProperty* prop);
void filenameChanged(QtProperty* prop);
void workspace_added(const QString &, Mantid::API::Workspace_sptr);
void workspace_removed(const QString &);
void currentItemChanged(QtBrowserItem*);
void addTie();
void addTieToFunction();
Expand Down
51 changes: 0 additions & 51 deletions Code/Mantid/MantidQt/MantidWidgets/src/FitPropertyBrowser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1520,44 +1520,6 @@ void FitPropertyBrowser::populateWorkspaceNames()
m_enumManager->setEnumNames(m_workspace, m_workspaceNames);
}

void FitPropertyBrowser::workspace_added(const QString &wsName, Mantid::API::Workspace_sptr ws)
{
if ( !isWorkspaceValid(ws) ) return;
QStringList oldWorkspaces = m_workspaceNames;
QString oldName = QString::fromStdString(workspaceName());
int i = m_workspaceNames.indexOf(wsName);
if (i < 0)
{
m_workspaceNames.append(wsName);
m_workspaceNames.sort();
}
m_enumManager->setEnumNames(m_workspace, m_workspaceNames);
i = m_workspaceNames.indexOf(oldName);
if (i >= 0)
{
m_enumManager->setValue(m_workspace,i);
}
getHandler()->updateWorkspaces(oldWorkspaces);
}

void FitPropertyBrowser::workspace_removed(const QString &wsName)
{
QStringList oldWorkspaces = m_workspaceNames;
QString oldName = QString::fromStdString(workspaceName());
int i = m_workspaceNames.indexOf(wsName);
if (i >= 0)
{
m_workspaceNames.removeAt(i);
}
m_enumManager->setEnumNames(m_workspace, m_workspaceNames);
i = m_workspaceNames.indexOf(oldName);
if (i >= 0)
{
m_enumManager->setValue(m_workspace,i);
}
getHandler()->updateWorkspaces(oldWorkspaces);
}

void FitPropertyBrowser::init()
{
populateFunctionNames();
Expand Down Expand Up @@ -2341,19 +2303,6 @@ void FitPropertyBrowser::setWorkspace(Mantid::API::IFitFunction* f)const
Mantid::API::AnalysisDataService::Instance().retrieve(wsName));
if (ws)
{
//int xMin=-1,xMax;
//double sX = startX();
//double eX = endX();
//const Mantid::MantidVec& X = ws->readX(workspaceIndex());
//for(xMax = 0;xMax < ws->blocksize(); ++xMax)
//{
// if (X[xMax] < sX) continue;
// else if (xMin < 0)
// {
// xMin = xMax;
// }
// if (X[xMax] > eX) break;
//}
QString slice = "WorkspaceIndex="+QString::number(workspaceIndex())+
",StartX="+QString::number(startX())+",EndX="+QString::number(endX());
f->setWorkspace(ws,slice.toStdString(),true);
Expand Down

0 comments on commit 0d80c20

Please sign in to comment.