Merge pull request #37839 from mantidproject/33252_storeInADS_preserved_in_alg_history

storeInADS preserved in algorithm history and recreated correctly by script builder

Author: SilkeSchomann

Framework/API/inc/MantidAPI/AlgorithmHistory.h

@@ -80,6 +80,8 @@ class MANTID_API_DLL AlgorithmHistory {
   const AlgorithmHistories &getChildHistories() const { return m_childHistories; }
   /// Retrieve a child algorithm history by index
   AlgorithmHistory_sptr getChildAlgorithmHistory(const size_t index) const;
+  /// get storeInADS
+  const bool &getStoreInADS() const { return m_storeInADS; }
   /// Add operator[] access
   AlgorithmHistory_sptr operator[](const size_t index) const;
   /// Retrieve the number of child algorithms
@@ -128,6 +130,8 @@ class MANTID_API_DLL AlgorithmHistory {
   AlgorithmHistories m_childHistories;
   /// UUID for this algorithm history
   std::string m_uuid;
+  /// If algorithm was set to store workspaces in the ADS
+  bool m_storeInADS{true};
 };
 
 MANTID_API_DLL std::ostream &operator<<(std::ostream &, const AlgorithmHistory &);

Framework/API/inc/MantidAPI/Workspace.h

@@ -67,9 +67,11 @@ class MANTID_API_DLL Workspace : public Kernel::DataItem {
 
   void virtual setTitle(const std::string &);
   void setComment(const std::string &);
+  void setPythonVariableName(const std::string &);
   virtual const std::string getTitle() const;
   const std::string &getComment() const;
   const std::string &getName() const override;
+  const std::string &getPythonVariableName() const;
   bool isDirty(const int n = 1) const;
   virtual bool isGroup() const { return false; }
   /// Get the footprint in memory in bytes.
@@ -95,6 +97,8 @@ class MANTID_API_DLL Workspace : public Kernel::DataItem {
   /// The name associated with the object within the ADS (This is required for
   /// workspace algebra
   std::string m_name;
+  /// The name of the variable holding the workspace, if not stored in the ADS
+  std::string m_pythonVariableName;
   /// The history of the workspace, algorithm and environment
   std::unique_ptr<WorkspaceHistory> m_history;
 

Framework/API/inc/MantidAPI/WorkspaceProperty.tcc

@@ -180,6 +180,9 @@ template <typename TYPE>
 std::string WorkspaceProperty<TYPE>::setDataItem(const std::shared_ptr<Kernel::DataItem> &value) {
   std::shared_ptr<TYPE> typed = std::dynamic_pointer_cast<TYPE>(value);
   if (typed) {
+    if (typed->getName().empty() && this->direction() == Kernel::Direction::Output) {
+      typed->setPythonVariableName(m_workspaceName);
+    }
     if (this->direction() == Kernel::Direction::Input) {
       m_workspaceName = typed->getName();
     }
@@ -299,15 +302,22 @@ template <typename TYPE> std::vector<std::string> WorkspaceProperty<TYPE>::allow
 template <typename TYPE> const Kernel::PropertyHistory WorkspaceProperty<TYPE>::createHistory() const {
   std::string wsName = m_workspaceName;
   bool isdefault = this->isDefault();
+  bool pythonVariable = false;
 
   if ((wsName.empty() || this->hasTemporaryValue()) && this->operator()()) {
-    // give the property a temporary name in the history
-    std::ostringstream os;
-    os << "__TMP" << this->operator()().get();
-    wsName = os.str();
+    const auto pvName = Kernel::PropertyWithValue<std::shared_ptr<TYPE>>::m_value->getPythonVariableName();
+    pythonVariable = !pvName.empty();
+    if (pythonVariable) {
+      wsName = pvName;
+    } else {
+      // give the property a temporary name in the history
+      std::ostringstream os;
+      os << "__TMP" << this->operator()().get();
+      wsName = os.str();
+    }
     isdefault = false;
   }
-  return Kernel::PropertyHistory(this->name(), wsName, this->type(), isdefault, this->direction());
+  return Kernel::PropertyHistory(this->name(), wsName, this->type(), isdefault, this->direction(), pythonVariable);
 }
 
 /** If this is an output workspace, store it into the AnalysisDataService

Framework/API/src/AlgorithmHistory.cpp

@@ -51,7 +51,7 @@ static boost::uuids::random_generator uuidGen;
 AlgorithmHistory::AlgorithmHistory(const Algorithm *const alg, const Types::Core::DateAndTime &start,
                                    const double &duration, std::size_t uexeccount)
     : m_name(alg->name()), m_version(alg->version()), m_executionDate(start), m_executionDuration(duration),
-      m_execCount(uexeccount), m_childHistories() {
+      m_execCount(uexeccount), m_childHistories(), m_storeInADS(alg->getAlwaysStoreInADS()) {
   // Now go through the algorithm's properties and create the PropertyHistory
   // objects.
   setProperties(alg);
@@ -109,6 +109,7 @@ void AlgorithmHistory::fillAlgorithmHistory(const Algorithm *const alg, const Ty
   m_executionDate = start;
   m_executionDuration = duration;
   m_execCount = uexeccount;
+  m_storeInADS = alg->getAlwaysStoreInADS();
   setProperties(alg);
 }
 
@@ -234,11 +235,13 @@ AlgorithmHistory &AlgorithmHistory::operator=(const AlgorithmHistory &A) {
     m_executionDate = A.m_executionDate;
     m_executionDuration = A.m_executionDuration;
     m_properties = A.m_properties;
+    m_storeInADS = A.m_storeInADS;
     // required to prevent destruction of descendant if assigning a descendant
     // to an ancestor
     auto temp = A.m_childHistories;
     m_childHistories = temp;
     m_uuid = A.m_uuid;
+    m_execCount = A.m_execCount;
   }
   return *this;
 }
@@ -271,7 +274,7 @@ void AlgorithmHistory::saveNexus(::NeXus::File *file, int &algCount) const {
   file->writeData("data", algData.str());
 
   // child algorithms
-  for (auto &history : m_childHistories) {
+  for (const auto &history : m_childHistories) {
     history->saveNexus(file, algCount);
   }
   file->closeGroup();

Framework/API/src/ScriptBuilder.cpp

@@ -206,7 +206,12 @@ const std::string ScriptBuilder::buildAlgorithmString(const AlgorithmHistory &al
     g_log.error() << "Could not create a fresh version of " << name << " version " << algHistory.version() << "\n";
   }
 
-  for (auto &propIter : props) {
+  const bool storeInADS = algHistory.getStoreInADS();
+
+  for (const auto &propIter : props) {
+    if (!storeInADS && propIter->name() == "OutputWorkspace") {
+      continue;
+    }
     std::string prop = buildPropertyString(*propIter, name);
     if (prop.length() > 0) {
       properties << prop << ", ";
@@ -233,14 +238,24 @@ const std::string ScriptBuilder::buildAlgorithmString(const AlgorithmHistory &al
   }
   // Third case is we never specify the version, so do nothing.
 
+  std::string assignmentStatement("");
+  if (!storeInADS) {
+    properties << "StoreInADS=False, ";
+    const auto it =
+        std::find_if(props.cbegin(), props.cend(), [](const auto &prop) { return prop->name() == "OutputWorkspace"; });
+    if (it != props.cend()) {
+      assignmentStatement = (*it)->value() + " = ";
+    }
+  }
+
   std::string propStr = properties.str();
   if (propStr.length() > 0) {
     // remove trailing comma & space
     propStr.erase(propStr.size() - 1);
     propStr.erase(propStr.size() - 1);
   }
 
-  std::string historyEntry = name + "(" + propStr + ")";
+  std::string historyEntry = assignmentStatement + name + "(" + propStr + ")";
   historyEntry.erase(boost::remove_if(historyEntry, boost::is_any_of("\n\r")), historyEntry.end());
   return historyEntry;
 }
@@ -286,11 +301,15 @@ const std::string ScriptBuilder::buildPropertyString(const Mantid::Kernel::Prope
       // Handle all other property types
     } else {
       std::string opener = "='";
+      std::string closer = "'";
       if (propHistory.value().find('\\') != std::string::npos) {
         opener = "=r'";
+      } else if (propHistory.pythonVariable()) {
+        opener = "=";
+        closer = "";
       }
 
-      prop = propHistory.name() + opener + propHistory.value() + "'";
+      prop = propHistory.name() + opener + propHistory.value() + closer;
     }
   }
 

Framework/API/src/Workspace.cpp

@@ -38,6 +38,8 @@ void Workspace::setComment(const std::string &c) { m_comment = c; }
  */
 void Workspace::setName(const std::string &name) { m_name = name; }
 
+void Workspace::setPythonVariableName(const std::string &name) { m_pythonVariableName = name; }
+
 /** Get the workspace title
  *
  *  @return The title
@@ -56,6 +58,8 @@ const std::string &Workspace::getComment() const { return m_comment; }
  */
 const std::string &Workspace::getName() const { return m_name; }
 
+const std::string &Workspace::getPythonVariableName() const { return m_pythonVariableName; }
+
 /**
  * Check whether other algorithms have been applied to the
  * workspace by checking the history length.
@@ -96,7 +100,7 @@ IPropertyManager::getValue<Mantid::API::Workspace_sptr>(const std::string &name)
 template <>
 MANTID_API_DLL Mantid::API::Workspace_const_sptr
 IPropertyManager::getValue<Mantid::API::Workspace_const_sptr>(const std::string &name) const {
-  auto *prop = dynamic_cast<PropertyWithValue<Mantid::API::Workspace_sptr> *>(getPointerToProperty(name));
+  const auto *prop = dynamic_cast<PropertyWithValue<Mantid::API::Workspace_sptr> *>(getPointerToProperty(name));
   if (prop) {
     return prop->operator()();
   } else {

Framework/API/test/ScriptBuilderTest.h

@@ -372,13 +372,13 @@ class ScriptBuilderTest : public CxxTest::TestSuite {
         "# Child algorithms of TopLevelAlgorithm",
         "",
         "## Child algorithms of NestedAlgorithm",
-        "BasicAlgorithm(PropertyA='FirstOne')",
-        "BasicAlgorithm(PropertyA='SecondOne')",
+        "BasicAlgorithm(PropertyA='FirstOne', StoreInADS=False)",
+        "BasicAlgorithm(PropertyA='SecondOne', StoreInADS=False)",
         "## End of child algorithms of NestedAlgorithm",
         "",
         "## Child algorithms of NestedAlgorithm",
-        "BasicAlgorithm(PropertyA='FirstOne')",
-        "BasicAlgorithm(PropertyA='SecondOne')",
+        "BasicAlgorithm(PropertyA='FirstOne', StoreInADS=False)",
+        "BasicAlgorithm(PropertyA='SecondOne', StoreInADS=False)",
         "## End of child algorithms of NestedAlgorithm",
         "",
         "# End of child algorithms of TopLevelAlgorithm",
@@ -419,16 +419,16 @@ class ScriptBuilderTest : public CxxTest::TestSuite {
         "# Child algorithms of TopLevelAlgorithm",
         "",
         "## Child algorithms of NestedAlgorithm",
-        "BasicAlgorithm(PropertyA='FirstOne')",
-        "BasicAlgorithm(PropertyA='SecondOne')",
+        "BasicAlgorithm(PropertyA='FirstOne', StoreInADS=False)",
+        "BasicAlgorithm(PropertyA='SecondOne', StoreInADS=False)",
         "## End of child algorithms of NestedAlgorithm",
         "",
-        "NestedAlgorithm()",
+        "NestedAlgorithm(StoreInADS=False)",
         "# End of child algorithms of TopLevelAlgorithm",
         "",
         "# Child algorithms of TopLevelAlgorithm",
-        "NestedAlgorithm()",
-        "NestedAlgorithm()",
+        "NestedAlgorithm(StoreInADS=False)",
+        "NestedAlgorithm(StoreInADS=False)",
         "# End of child algorithms of TopLevelAlgorithm",
         "",
         "",
@@ -604,6 +604,41 @@ class ScriptBuilderTest : public CxxTest::TestSuite {
     m_ads.remove("IRS21360");
   }
 
+  void test_ScriptBuilderWithOutputWorkspaceOutsideOfADS() {
+    std::vector<double> xData = {1, 2, 3};
+    std::vector<double> yData = {1, 2, 3};
+
+    auto createWorkspaceAlg = m_algFactory.create("CreateWorkspace", 1);
+    createWorkspaceAlg->initialize();
+    createWorkspaceAlg->setProperty("DataX", xData);
+    createWorkspaceAlg->setProperty("DataY", yData);
+    createWorkspaceAlg->setProperty("OutputWorkspace", "ws");
+    createWorkspaceAlg->setAlwaysStoreInADS(false);
+    createWorkspaceAlg->execute();
+
+    MatrixWorkspace_sptr ws = createWorkspaceAlg->getProperty("OutputWorkspace");
+    std::vector<double> params = {1, 3, 10};
+    auto rebinAlg = m_algFactory.create("Rebin", 1);
+    rebinAlg->initialize();
+    rebinAlg->setProperty("InputWorkspace", ws);
+    rebinAlg->setProperty("Params", params);
+    rebinAlg->setProperty("Power", 0.5);
+    rebinAlg->setProperty("OutputWorkspace", "result");
+    rebinAlg->execute();
+
+    auto resultWs = m_ads.retrieveWS<MatrixWorkspace>("result");
+    auto wsHist = resultWs->getHistory();
+
+    ScriptBuilder builder(wsHist.createView());
+    const std::string scriptText = builder.build();
+    const std::string expectedCreateWorkspaceLine =
+        "ws = CreateWorkspace(DataX='1,2,3', DataY='1,2,3', StoreInADS=False)";
+    const std::string expectedRebinLine =
+        "Rebin(InputWorkspace=ws, OutputWorkspace='result', Params='1,3,10', Power=0.5)";
+    TS_ASSERT(scriptText.find(expectedCreateWorkspaceLine) != std::string::npos);
+    TS_ASSERT(scriptText.find(expectedRebinLine) != std::string::npos);
+  }
+
 private:
   AlgorithmFactoryImpl &m_algFactory;
   AnalysisDataServiceImpl &m_ads;

Framework/Kernel/inc/MantidKernel/PropertyHistory.h

@@ -34,7 +34,7 @@ class Property;
 class MANTID_KERNEL_DLL PropertyHistory {
 public:
   PropertyHistory(std::string name, std::string value, std::string type, const bool isdefault,
-                  const unsigned int direction = 99);
+                  const unsigned int direction = 99, const bool pythonVariable = false);
 
   /// construct a property history from a property object
   PropertyHistory(Property const *const prop);
@@ -57,6 +57,7 @@ class MANTID_KERNEL_DLL PropertyHistory {
   /// get whether algorithm parameter was left as default EMPTY_INT,LONG,DBL
   /// const
   bool isEmptyDefault() const;
+  bool pythonVariable() const { return m_pythonVariable; };
 
   /// this is required for boost.python
   bool operator==(const PropertyHistory &other) const {
@@ -75,6 +76,8 @@ class MANTID_KERNEL_DLL PropertyHistory {
   bool m_isDefault;
   /// direction of parameter
   unsigned int m_direction;
+  /// Whether the property should be treated as a python variable instead of string when building a script from history
+  bool m_pythonVariable;
 };
 
 // typedefs for property history pointers

Framework/Kernel/src/PropertyHistory.cpp

@@ -22,13 +22,13 @@ namespace Mantid::Kernel {
 
 /// Constructor
 PropertyHistory::PropertyHistory(std::string name, std::string value, std::string type, const bool isdefault,
-                                 const unsigned int direction)
+                                 const unsigned int direction, const bool pythonVariable)
     : m_name(std::move(name)), m_value(std::move(value)), m_type(std::move(type)), m_isDefault(isdefault),
-      m_direction(direction) {}
+      m_direction(direction), m_pythonVariable(pythonVariable) {}
 
 PropertyHistory::PropertyHistory(Property const *const prop)
     : m_name(prop->name()), m_value(prop->valueAsPrettyStr(0, true)), m_type(prop->type()),
-      m_isDefault(prop->isDefault()), m_direction(prop->direction()) {}
+      m_isDefault(prop->isDefault()), m_direction(prop->direction()), m_pythonVariable(false) {}
 
 /** Prints a text representation of itself
  *  @param os :: The output stream to write to

Framework/PythonInterface/mantid/api/src/Exports/AlgorithmHistory.cpp

@@ -87,6 +87,9 @@ void export_AlgorithmHistory() {
 
       .def("getChildAlgorithm", &AlgorithmHistory::getChildAlgorithm, (arg("self"), arg("index")),
            "Returns the algorithm at the given index in the history")
+
+      .def("getStoreInADS", &AlgorithmHistory::getStoreInADS, arg("self"), return_value_policy<copy_const_reference>(),
+           "Return storeInADS property")
       // ----------------- Operators --------------------------------------
       .def(self_ns::str(self));
 }