diff --git a/qt/widgets/common/inc/MantidQtWidgets/Common/FitScriptGeneratorModel.h b/qt/widgets/common/inc/MantidQtWidgets/Common/FitScriptGeneratorModel.h index c1227e21399782c14c9a9b63adcffbf3b14b8464..68290401dfa8a54ed51cc0585679b6487cc4b4cc 100644 --- a/qt/widgets/common/inc/MantidQtWidgets/Common/FitScriptGeneratorModel.h +++ b/qt/widgets/common/inc/MantidQtWidgets/Common/FitScriptGeneratorModel.h @@ -153,6 +153,7 @@ private: getParameterValue(FitDomainIndex domainIndex, std::string const &fullParameter) const; + [[nodiscard]] bool validParameter(std::string const &fullParameter) const; [[nodiscard]] bool validParameter(FitDomainIndex domainIndex, std::string const &fullParameter) const; [[nodiscard]] bool validTie(std::string const &fullTie) const; @@ -178,6 +179,9 @@ private: void checkGlobalParameterhasNoTies(std::string const &globalParameter) const; void checkParameterIsNotGlobal(std::string const &fullParameter) const; + void tryToAdjustParameterInGlobalTieIfInvalidated(GlobalTie &globalTie); + void tryToAdjustTieInGlobalTieIfInvalidated(GlobalTie &globalTie); + IFitScriptGeneratorPresenter *m_presenter; std::vector<std::unique_ptr<FitDomain>> m_fitDomains; // A vector of global parameters. E.g. f0.A0 diff --git a/qt/widgets/common/inc/MantidQtWidgets/Common/FittingGlobals.h b/qt/widgets/common/inc/MantidQtWidgets/Common/FittingGlobals.h index dfc2f0bd7e84ede5ed554b4f4d72924fd935f893..1161d7bfc791becdaffb47d5e4b954c121115499 100644 --- a/qt/widgets/common/inc/MantidQtWidgets/Common/FittingGlobals.h +++ b/qt/widgets/common/inc/MantidQtWidgets/Common/FittingGlobals.h @@ -38,6 +38,9 @@ struct EXPORT_OPT_MANTIDQT_COMMON GlobalTie { GlobalTie(std::string const ¶meter, std::string const &tie); + std::string toCompositeParameter(std ::string const &fullParameter) const; + std::string toNonCompositeParameter(std ::string const &fullParameter) const; + std::string m_parameter; std::string m_tie; }; diff --git a/qt/widgets/common/src/FitDomain.cpp b/qt/widgets/common/src/FitDomain.cpp index 55336198e07218d4235b67c4fd638bb23d07c623..cb47643af4e283ae3b561cda75babe86ae95d299 100644 --- a/qt/widgets/common/src/FitDomain.cpp +++ b/qt/widgets/common/src/FitDomain.cpp @@ -283,6 +283,9 @@ void FitDomain::appendParametersTiedTo( bool FitDomain::isParameterValueWithinConstraints(std::string const ¶meter, double value) const { + if (!hasParameter(parameter)) + return false; + auto isValid = true; auto const parameterIndex = m_function->parameterIndex(parameter); diff --git a/qt/widgets/common/src/FitScriptGeneratorModel.cpp b/qt/widgets/common/src/FitScriptGeneratorModel.cpp index 1f948f29148162c3408d0cac520f83c79241d642..01af27e4a3200c2f43c8611e5f326fe18f54904a 100644 --- a/qt/widgets/common/src/FitScriptGeneratorModel.cpp +++ b/qt/widgets/common/src/FitScriptGeneratorModel.cpp @@ -367,6 +367,11 @@ double FitScriptGeneratorModel::getParameterValue( throw std::runtime_error("The domain index provided does not exist."); } +bool FitScriptGeneratorModel::validParameter( + std::string const &fullParameter) const { + return validParameter(getDomainIndexOf(fullParameter), fullParameter); +} + bool FitScriptGeneratorModel::validParameter( FitDomainIndex domainIndex, std::string const &fullParameter) const { if (domainIndex.value < numberOfDomains()) { @@ -395,8 +400,7 @@ bool FitScriptGeneratorModel::validTie(std::string const &tie) const { bool FitScriptGeneratorModel::validGlobalTie(std::string const &fullParameter, std::string const &fullTie) const { - if (!fullTie.empty() && !isNumber(fullTie) && - validParameter(getDomainIndexOf(fullTie), fullTie)) { + if (!fullTie.empty() && !isNumber(fullTie) && validParameter(fullTie)) { auto const domainIndex = getDomainIndexOf(fullParameter); auto const tieDomainIndex = getDomainIndexOf(fullTie); return isParameterValueWithinConstraints( @@ -425,18 +429,50 @@ void FitScriptGeneratorModel::clearGlobalTie(std::string const &fullParameter) { } void FitScriptGeneratorModel::checkGlobalTies() { - auto const isTieInvalid = [&](GlobalTie const &globalTie) { - return !validParameter(getDomainIndexOf(globalTie.m_parameter), - globalTie.m_parameter) || + auto const isTieInvalid = [&](GlobalTie &globalTie) { + tryToAdjustParameterInGlobalTieIfInvalidated(globalTie); + tryToAdjustTieInGlobalTieIfInvalidated(globalTie); + + return !validParameter(globalTie.m_parameter) || !validGlobalTie(globalTie.m_parameter, globalTie.m_tie); }; auto const iter = std::remove_if(m_globalTies.begin(), m_globalTies.end(), isTieInvalid); - if (iter != m_globalTies.cend()) { + if (iter != m_globalTies.cend()) m_globalTies.erase(iter, m_globalTies.cend()); - m_presenter->setGlobalTies(m_globalTies); + + m_presenter->setGlobalTies(m_globalTies); +} + +void FitScriptGeneratorModel::tryToAdjustParameterInGlobalTieIfInvalidated( + GlobalTie &globalTie) { + if (!validParameter(globalTie.m_parameter)) { + auto const promotedParam = + globalTie.toCompositeParameter(globalTie.m_parameter); + auto const demotedParam = + globalTie.toNonCompositeParameter(globalTie.m_parameter); + + if (validParameter(promotedParam)) { + globalTie.m_parameter = promotedParam; + } else if (validParameter(demotedParam)) { + globalTie.m_parameter = demotedParam; + } + } +} + +void FitScriptGeneratorModel::tryToAdjustTieInGlobalTieIfInvalidated( + GlobalTie &globalTie) { + if (!validGlobalTie(globalTie.m_parameter, globalTie.m_tie)) { + auto const promotedTie = globalTie.toCompositeParameter(globalTie.m_tie); + auto const demotedTie = globalTie.toNonCompositeParameter(globalTie.m_tie); + + if (validGlobalTie(globalTie.m_parameter, promotedTie)) { + globalTie.m_tie = promotedTie; + } else if (validGlobalTie(globalTie.m_parameter, demotedTie)) { + globalTie.m_tie = demotedTie; + } } } diff --git a/qt/widgets/common/src/FittingGlobals.cpp b/qt/widgets/common/src/FittingGlobals.cpp index cc04ef66d054459739a3548e446234b49bb86fa3..70f3f78ba1f3b1ec512969bb9df573c4f4b8b587 100644 --- a/qt/widgets/common/src/FittingGlobals.cpp +++ b/qt/widgets/common/src/FittingGlobals.cpp @@ -15,5 +15,28 @@ GlobalParameter::GlobalParameter(std::string const ¶meter) GlobalTie::GlobalTie(std::string const ¶meter, std::string const &tie) : m_parameter(parameter), m_tie(tie) {} +std::string +GlobalTie::toCompositeParameter(std ::string const &fullParameter) const { + auto const dotIndex = fullParameter.find("."); + if (dotIndex != std::string::npos) { + return fullParameter.substr(0, dotIndex + 1) + "f0." + + fullParameter.substr(dotIndex + 1); + } + return fullParameter; +} + +std::string +GlobalTie::toNonCompositeParameter(std ::string const &fullParameter) const { + auto const firstDotIndex = fullParameter.find("."); + if (firstDotIndex != std::string::npos) { + auto const secondDotIndex = fullParameter.find(".", firstDotIndex + 1); + if (secondDotIndex != std::string::npos) { + return fullParameter.substr(0, firstDotIndex + 1) + + fullParameter.substr(secondDotIndex + 1); + } + } + return fullParameter; +} + } // namespace MantidWidgets } // namespace MantidQt diff --git a/qt/widgets/common/src/FunctionTreeView.cpp b/qt/widgets/common/src/FunctionTreeView.cpp index 8ac9fb59796295474e8f5776ed39af71a3747e0e..1aa9f4a0dd7ba67d0cfce88d76df7097343ffa95 100644 --- a/qt/widgets/common/src/FunctionTreeView.cpp +++ b/qt/widgets/common/src/FunctionTreeView.cpp @@ -56,15 +56,32 @@ using namespace Mantid::API; namespace { const char *globalOptionName = "Global"; Mantid::Kernel::Logger g_log("Function Browser"); -QString addPrefix(QString ¶m) { return QString("f0.") + param; } const std::regex PREFIX_REGEX("(^[f][0-9](.*))"); inline bool variableIsPrefixed(const std::string &name) { return std::regex_match(name, PREFIX_REGEX); } -QString removePrefix(QString ¶m) { - if (variableIsPrefixed(param.toStdString())) - return param.split(QString("."))[1]; - else { + +QString insertPrefix(const QString ¶m) { + return param.left(param.indexOf(".") + 1) + "f0." + + param.right(param.size() - param.indexOf(".") - 1); +} + +QString addPrefix(const QString ¶m) { return "f0." + param; } + +QString removeEmbeddedPrefix(const QString ¶m) { + if (variableIsPrefixed(param.toStdString())) { + const auto paramSplit = param.split("."); + return paramSplit[0] + "." + paramSplit[paramSplit.size() - 1]; + } else { + return param; + } +} + +QString removePrefix(const QString ¶m) { + if (variableIsPrefixed(param.toStdString())) { + const auto paramSplit = param.split("."); + return paramSplit[paramSplit.size() - 1]; + } else { return param; } } @@ -1418,7 +1435,9 @@ void FunctionTreeView::addFunctionEnd(int result) { if (f0) { // Modify the previous globals so they have a function prefix std::transform(globalParameters.begin(), globalParameters.end(), - globalParameters.begin(), addPrefix); + globalParameters.begin(), + m_multiDomainFunctionPrefix.isEmpty() ? addPrefix + : insertPrefix); cf->addFunction(f0); } cf->addFunction(f); @@ -1769,21 +1788,22 @@ void FunctionTreeView::removeFunction() { // Check if the current function in the browser is a // composite function auto topProp = props[0]; - auto fun = Mantid::API::FunctionFactory::Instance().createFunction( - topProp->propertyName().toStdString()); - auto cf = std::dynamic_pointer_cast<Mantid::API::CompositeFunction>(fun); + auto cf = std::dynamic_pointer_cast<Mantid::API::CompositeFunction>( + getFunction(topProp)); if (cf) { // If it is a composite function // check that there are more than one function // which means more than two subproperties - size_t nFunctions = props[0]->subProperties().size() - 1; - if (nFunctions == 1 && cf->name() == "CompositeFunction") { + if (cf->nFunctions() == 1 && cf->name() == "CompositeFunction") { // If only one function remains, remove the composite function: // Temporary copy the remaining function auto func = getFunction(props[0]->subProperties()[1]); std::transform(globalParameters.begin(), globalParameters.end(), - globalParameters.begin(), removePrefix); + globalParameters.begin(), + m_multiDomainFunctionPrefix.isEmpty() + ? removePrefix + : removeEmbeddedPrefix); // Remove the composite function m_browser->removeProperty(topProp); // Add the temporary stored function diff --git a/qt/widgets/common/test/FitScriptGeneratorModelTest.h b/qt/widgets/common/test/FitScriptGeneratorModelTest.h index cae56832589807f60ca9e02262d35da5d7d2bff7..11d14fba76b1ed14609fc1a919d6d09d72a81d69 100644 --- a/qt/widgets/common/test/FitScriptGeneratorModelTest.h +++ b/qt/widgets/common/test/FitScriptGeneratorModelTest.h @@ -212,12 +212,24 @@ public: m_flatBackground->asString()); } - void test_that_addFunction_will_clear_the_global_ties_that_have_expired() { + void + test_that_addFunction_will_dynamically_adjust_the_global_ties_that_have_changed_function_index() { setup_simultaneous_fit_with_global_tie(); - TS_ASSERT_EQUALS(m_model->getGlobalTies().size(), 1); + auto const globalTiesBefore = m_model->getGlobalTies(); + TS_ASSERT_EQUALS(globalTiesBefore.size(), 1); + TS_ASSERT_EQUALS(globalTiesBefore[0].m_parameter, "f0.A0"); + TS_ASSERT_EQUALS(globalTiesBefore[0].m_tie, "f1.A0"); + + // Add a function (thereby creating a composite) m_model->addFunction(m_wsName, m_wsIndex, m_expDecay->asString()); - TS_ASSERT_EQUALS(m_model->getGlobalTies().size(), 0); + m_model->addFunction("Name2", m_wsIndex, m_expDecay->asString()); + + // The global tie has shifted up one index because it is now a composite. + auto const globalTiesAfter = m_model->getGlobalTies(); + TS_ASSERT_EQUALS(globalTiesAfter.size(), 1); + TS_ASSERT_EQUALS(globalTiesAfter[0].m_parameter, "f0.f0.A0"); + TS_ASSERT_EQUALS(globalTiesAfter[0].m_tie, "f1.f0.A0"); } void test_that_addFunction_will_throw_if_provided_a_composite_function() { @@ -280,9 +292,36 @@ public: TS_ASSERT_EQUALS(m_model->getGlobalTies().size(), 1); m_model->removeFunction(m_wsName, m_wsIndex, m_flatBackground->asString()); + m_model->removeFunction("Name2", m_wsIndex, m_flatBackground->asString()); TS_ASSERT_EQUALS(m_model->getGlobalTies().size(), 0); } + void + test_that_removeFunction_will_dynamically_adjust_the_global_ties_that_have_changed_function_index() { + setup_simultaneous_fit_with_no_ties(); + + // Add a function to create a composite + m_model->addFunction(m_wsName, m_wsIndex, m_expDecay->asString()); + m_model->addFunction("Name2", m_wsIndex, m_expDecay->asString()); + m_model->updateParameterTie("Name2", m_wsIndex, "f1.f1.Height", + "f0.f1.Height"); + + auto const globalTiesBefore = m_model->getGlobalTies(); + TS_ASSERT_EQUALS(globalTiesBefore.size(), 1); + TS_ASSERT_EQUALS(globalTiesBefore[0].m_parameter, "f1.f1.Height"); + TS_ASSERT_EQUALS(globalTiesBefore[0].m_tie, "f0.f1.Height"); + + // Remove the flat background (thereby elimating the need for a composite) + m_model->removeFunction(m_wsName, m_wsIndex, m_flatBackground->asString()); + m_model->removeFunction("Name2", m_wsIndex, m_flatBackground->asString()); + + // The global tie has shifted down one index because the composite is gone. + auto const globalTiesAfter = m_model->getGlobalTies(); + TS_ASSERT_EQUALS(globalTiesAfter.size(), 1); + TS_ASSERT_EQUALS(globalTiesAfter[0].m_parameter, "f1.Height"); + TS_ASSERT_EQUALS(globalTiesAfter[0].m_tie, "f0.Height"); + } + void test_that_getFunction_will_throw_if_the_domain_specified_does_not_exist() { TS_ASSERT_THROWS(m_model->getFunction(m_wsName, m_wsIndex),