diff --git a/Framework/API/inc/MantidAPI/AlgorithmManager.h b/Framework/API/inc/MantidAPI/AlgorithmManager.h index 3ad91d867e6d5bf6615f76b8293db63289dc8202..652795dd2ac45ca16480cbd93a7c6fd83ad1e4ce 100644 --- a/Framework/API/inc/MantidAPI/AlgorithmManager.h +++ b/Framework/API/inc/MantidAPI/AlgorithmManager.h @@ -49,7 +49,6 @@ public: const int &version = -1) const; std::size_t size() const; - void setMaxAlgorithms(int n); IAlgorithm_sptr getAlgorithm(AlgorithmID id) const; void removeById(AlgorithmID id); @@ -79,8 +78,9 @@ private: /// Unimplemented assignment operator AlgorithmManagerImpl &operator=(const AlgorithmManagerImpl &); - /// The maximum size of the algorithm store - int m_max_no_algs; + /// Removes any finished algorithms from the list of managed algorithms + size_t removeFinishedAlgorithms(); + /// The list of managed algorithms std::deque<IAlgorithm_sptr> m_managed_algs; ///< pointers to managed algorithms [policy???] diff --git a/Framework/API/inc/MantidAPI/IAlgorithm.h b/Framework/API/inc/MantidAPI/IAlgorithm.h index 1baf1b5e395b57243342ca5198d9016b25b31a02..9b22aeaa403a35904c47c304c99f68b1bad32d24 100644 --- a/Framework/API/inc/MantidAPI/IAlgorithm.h +++ b/Framework/API/inc/MantidAPI/IAlgorithm.h @@ -122,7 +122,7 @@ public: /// Check whether the algorithm is initialized properly virtual bool isInitialized() const = 0; - /// Check whether the algorithm has already been executed + /// Check whether the algorithm has been executed sucessfully virtual bool isExecuted() const = 0; /// Raises the cancel flag. interuption_point() method if called inside exec() diff --git a/Framework/API/src/Algorithm.cpp b/Framework/API/src/Algorithm.cpp index 8a96b5f46197db6fb9c111209e5daba26c2e241b..f76b90be2ee3db4a413cd4a91257dceb6ddbc174 100644 --- a/Framework/API/src/Algorithm.cpp +++ b/Framework/API/src/Algorithm.cpp @@ -142,7 +142,7 @@ bool Algorithm::isInitialized() const { return (m_executionState != ExecutionState::Uninitialized); } -/// Has the Algorithm already been executed +/// Has the Algorithm already been executed successfully bool Algorithm::isExecuted() const { return ((executionState() == ExecutionState::Finished) && (resultState() == ResultState::Success)); diff --git a/Framework/API/src/AlgorithmManager.cpp b/Framework/API/src/AlgorithmManager.cpp index 8c2d63386975bc5e2e61b6aef3193ebec355d305..2d657d81c4ffd3abba445d1597ce12ae57755968 100644 --- a/Framework/API/src/AlgorithmManager.cpp +++ b/Framework/API/src/AlgorithmManager.cpp @@ -22,15 +22,6 @@ Kernel::Logger g_log("AlgorithmManager"); /// Private Constructor for singleton class AlgorithmManagerImpl::AlgorithmManagerImpl() : m_managed_algs() { - auto max_no_algs = - Kernel::ConfigService::Instance().getValue<int>("algorithms.retained"); - - m_max_no_algs = max_no_algs.get_value_or(0); - - if (m_max_no_algs < 1) { - m_max_no_algs = 100; // Default to keeping 100 algorithms if not specified - } - g_log.debug() << "Algorithm Manager created.\n"; } @@ -82,39 +73,15 @@ IAlgorithm_sptr AlgorithmManagerImpl::create(const std::string &algName, else alg = unmanagedAlg; - // If this takes us beyond the maximum size, then remove the oldest one(s) - while (m_managed_algs.size() >= - static_cast<std::deque<IAlgorithm_sptr>::size_type>(m_max_no_algs)) { - std::deque<IAlgorithm_sptr>::iterator it; - it = m_managed_algs.begin(); - - // Look for the first (oldest) algo that is NOT running right now. - while (it != m_managed_algs.end()) { - if (!(*it)->isRunning()) - break; - ++it; - } - - if (it == m_managed_algs.end()) { - // Unusual case where ALL algorithms are running - g_log.warning() - << "All algorithms in the AlgorithmManager are running. " - << "Cannot pop oldest algorithm. " - << "You should increase your 'algorithms.retained' value. " - << m_managed_algs.size() << " in queue.\n"; - break; - } else { - // Normal; erase that algorithm - g_log.debug() << "Popping out oldest algorithm " << (*it)->name() - << '\n'; - m_managed_algs.erase(it); - } - } + auto count = removeFinishedAlgorithms(); + g_log.debug() + << count + << " Finished algorithms removed from the managed algorithms list. " + << m_managed_algs.size() << " remaining.\n"; // Add to list of managed ones m_managed_algs.emplace_back(alg); alg->initialize(); - } catch (std::runtime_error &ex) { g_log.error() << "AlgorithmManager:: Unable to create algorithm " << algName << ' ' << ex.what() << '\n'; @@ -140,19 +107,6 @@ void AlgorithmManagerImpl::clear() { std::size_t AlgorithmManagerImpl::size() const { return m_managed_algs.size(); } -/** - * Set new maximum number of algorithms that can be stored. - * - * @param n :: The new maximum. - */ -void AlgorithmManagerImpl::setMaxAlgorithms(int n) { - if (n < 0) { - throw std::runtime_error("Maximum number of algorithms stored in " - "AlgorithmManager cannot be negative."); - } - m_max_no_algs = n; -} - /** * Returns a shared pointer by algorithm id * @param id :: The ID of the algorithm @@ -241,6 +195,28 @@ void AlgorithmManagerImpl::cancelAll() { } } +/// Removes all of the finished algorithms +/// this does not lock the mutex as the locking is already assumed to be in +/// place +size_t AlgorithmManagerImpl::removeFinishedAlgorithms() { + std::vector<IAlgorithm_const_sptr> theCompletedInstances; + std::copy_if( + m_managed_algs.cbegin(), m_managed_algs.cend(), + std::back_inserter(theCompletedInstances), [](const auto &algorithm) { + return (algorithm->executionState() == ExecutionState::Finished); + }); + for (auto completedAlg : theCompletedInstances) { + auto itend = m_managed_algs.end(); + for (auto it = m_managed_algs.begin(); it != itend; ++it) { + if ((**it).getAlgorithmID() == completedAlg->getAlgorithmID()) { + m_managed_algs.erase(it); + break; + } + } + } + return theCompletedInstances.size(); +} + void AlgorithmManagerImpl::shutdown() { clear(); } } // namespace API } // namespace Mantid diff --git a/Framework/API/test/AlgorithmManagerTest.h b/Framework/API/test/AlgorithmManagerTest.h index e20adb182f740bde8237960592d144271caf213a..e33bd369e99f41f277ef33a6cdebb86d6f341424 100644 --- a/Framework/API/test/AlgorithmManagerTest.h +++ b/Framework/API/test/AlgorithmManagerTest.h @@ -106,12 +106,6 @@ public: } static void destroySuite(AlgorithmManagerTest *suite) { delete suite; } - AlgorithmManagerTest() { - // A test fails unless algorithms.retained is big enough - Mantid::Kernel::ConfigService::Instance().setString("algorithms.retained", - "5"); - } - void testVersionFail() { const size_t nalgs = AlgorithmFactory::Instance().getKeys().size(); TS_ASSERT_THROWS(AlgorithmFactory::Instance().subscribe<AlgTestFail>(), @@ -217,97 +211,6 @@ public: AlgorithmManager::Instance().notificationCenter.removeObserver(my_observer); } - /** Keep the right number of algorithms in the list. - * This also tests setMaxAlgorithms(). - */ - void testDroppingOldOnes() { - AlgorithmManager::Instance().setMaxAlgorithms(5); - AlgorithmManager::Instance().clear(); - TS_ASSERT_EQUALS(AlgorithmManager::Instance().size(), 0); - - IAlgorithm_sptr first = AlgorithmManager::Instance().create("AlgTest"); - // Fill up the list - for (size_t i = 1; i < 5; i++) - AlgorithmManager::Instance().create("AlgTest"); - TS_ASSERT_EQUALS(AlgorithmManager::Instance().size(), 5); - - // The first one is still in the list - TS_ASSERT(AlgorithmManager::Instance().getAlgorithm( - first->getAlgorithmID()) == first); - - // Add one more, drops the oldest one - AlgorithmManager::Instance().create("AlgTest"); - TS_ASSERT_EQUALS(AlgorithmManager::Instance().size(), 5); - TS_ASSERT( - !AlgorithmManager::Instance().getAlgorithm(first->getAlgorithmID())); - } - - /** Keep one algorithm running, drop the second-oldest one etc. */ - void testDroppingOldOnes_whenAnAlgorithmIsStillRunning() { - AlgorithmManager::Instance().clear(); - TS_ASSERT_EQUALS(AlgorithmManager::Instance().size(), 0); - - // Create one algorithm that appears never to stop - IAlgorithm_sptr first = - AlgorithmManager::Instance().create("AlgRunsForever"); - - IAlgorithm_sptr second = AlgorithmManager::Instance().create("AlgTest"); - - // Another long-running algo - IAlgorithm_sptr third = - AlgorithmManager::Instance().create("AlgRunsForever"); - - for (size_t i = 3; i < 5; i++) - AlgorithmManager::Instance().create("AlgTest"); - TS_ASSERT_EQUALS(AlgorithmManager::Instance().size(), 5); - - // The first three created are in the list - TS_ASSERT(AlgorithmManager::Instance().getAlgorithm( - first->getAlgorithmID()) == first); - TS_ASSERT(AlgorithmManager::Instance().getAlgorithm( - second->getAlgorithmID()) == second); - TS_ASSERT(AlgorithmManager::Instance().getAlgorithm( - third->getAlgorithmID()) == third); - - // Add one more, drops the SECOND oldest one - AlgorithmManager::Instance().create("AlgTest"); - TS_ASSERT_EQUALS(AlgorithmManager::Instance().size(), 5); - - TSM_ASSERT("The oldest algorithm (is still running) so it is still there", - AlgorithmManager::Instance().getAlgorithm( - first->getAlgorithmID()) == first); - TSM_ASSERT( - "The second oldest was popped, so trying to get it should return null", - !AlgorithmManager::Instance().getAlgorithm(second->getAlgorithmID())); - - // One more time - AlgorithmManager::Instance().create("AlgTest"); - TS_ASSERT_EQUALS(AlgorithmManager::Instance().size(), 5); - - // The right ones are at the front - TSM_ASSERT("The oldest algorithm (is still running) so it is still there", - AlgorithmManager::Instance().getAlgorithm( - first->getAlgorithmID()) == first); - TSM_ASSERT("The third algorithm (is still running) so it is still there", - AlgorithmManager::Instance().getAlgorithm( - third->getAlgorithmID()) == third); - AlgorithmManager::Instance().cancelAll(); - } - - void testDroppingOldOnes_extremeCase() { - /** Extreme case where your queue fills up and all algos are running */ - AlgorithmManager::Instance().clear(); - for (size_t i = 0; i < 5; i++) { - AlgorithmManager::Instance().create("AlgRunsForever"); - } - - TS_ASSERT_EQUALS(AlgorithmManager::Instance().size(), 5); - // Create another that takes it past the normal max size (of 5) - AlgorithmManager::Instance().create("AlgTest"); - TS_ASSERT_EQUALS(AlgorithmManager::Instance().size(), 6); - AlgorithmManager::Instance().cancelAll(); - } - void testThreadSafety() { PARALLEL_FOR_NO_WSP_CHECK() for (int i = 0; i < 5000; i++) { @@ -317,7 +220,6 @@ public: void testRemovingByIdRemovesCorrectObject() { auto &mgr = AlgorithmManager::Instance(); - mgr.setMaxAlgorithms(10); const size_t initialManagerSize = mgr.size(); // 2 different ids for same named algorithm auto alg1 = mgr.create("AlgTest"); @@ -329,8 +231,6 @@ public: // the right one? auto foundAlg = mgr.getAlgorithm(alg2->getAlgorithmID()); TS_ASSERT(foundAlg); - - mgr.setMaxAlgorithms(5); } void test_runningInstancesOf() { diff --git a/Framework/Kernel/src/ConfigService.cpp b/Framework/Kernel/src/ConfigService.cpp index 5e6b014115aa8b36c639bfab121eb149a1396457..fcd12bc765e9b373f7f570eb25591c7ab2bb5349 100644 --- a/Framework/Kernel/src/ConfigService.cpp +++ b/Framework/Kernel/src/ConfigService.cpp @@ -651,8 +651,6 @@ void ConfigServiceImpl::createUserPropertiesFile() const { filestr << "##\n"; filestr << "## GENERAL\n"; filestr << "##\n\n"; - filestr << "## Set the number of algorithm properties to retain\n"; - filestr << "#algorithms.retained=90\n\n"; filestr << "## Set the maximum number of cores used to run algorithms over\n"; filestr << "#MultiThreaded.MaxCores=4\n\n"; diff --git a/Framework/Kernel/test/ConfigPropertyObserverTest.h b/Framework/Kernel/test/ConfigPropertyObserverTest.h index 941b29a01b4eee8b48033402c2d8523df87d6185..9982dbd761b2a028d362ebf532ce475e1c2c1f2a 100644 --- a/Framework/Kernel/test/ConfigPropertyObserverTest.h +++ b/Framework/Kernel/test/ConfigPropertyObserverTest.h @@ -41,8 +41,6 @@ public: ConfigService::Instance().getString("datasearch.directories"); m_defaultSaveDirectory = ConfigService::Instance().getString("defaultsave.directory"); - m_retainedAlgorithms = - ConfigService::Instance().getString("algorithms.retained"); } void tearDown() override { @@ -50,8 +48,6 @@ public: m_searchDirectories); ConfigService::Instance().setString("defaultsave.directory", m_defaultSaveDirectory); - ConfigService::Instance().setString("algorithms.retained", - m_retainedAlgorithms); } void testRecievesCallbackForSearchDirectoryChange() { @@ -96,7 +92,7 @@ public: }); auto callCountB = 0; auto observerB = - makeMockObserver("algorithms.retained", + makeMockObserver("projectRecovery.secondsBetween", [&callCountB](const std::string &newValue, const std::string &prevValue) -> void { UNUSED_ARG(newValue); @@ -105,7 +101,8 @@ public: }); ConfigService::Instance().setString("datasearch.directories", "/dev/null"); - ConfigService::Instance().setString("algorithms.retained", "40"); + ConfigService::Instance().setString("projectRecovery.secondsBetween", + "600"); TS_ASSERT_EQUALS(1, callCountA); TS_ASSERT_EQUALS(1, callCountB); diff --git a/Framework/Kernel/test/ConfigServiceTest.h b/Framework/Kernel/test/ConfigServiceTest.h index cacccda23a13172c89ef521d0821b2a2bbc7eda3..384476a09cbc3ef59b4fe75bba5527caf10e7485 100644 --- a/Framework/Kernel/test/ConfigServiceTest.h +++ b/Framework/Kernel/test/ConfigServiceTest.h @@ -329,21 +329,20 @@ public: void TestCustomProperty() { std::string countString = - ConfigService::Instance().getString("algorithms.retained"); - TS_ASSERT_EQUALS(countString, "50"); + ConfigService::Instance().getString("projectRecovery.secondsBetween"); + TS_ASSERT_EQUALS(countString, "60"); } void TestCustomPropertyAsValue() { - // Mantid.legs is defined in the properties script as 6 int value = ConfigService::Instance() - .getValue<int>("algorithms.retained") + .getValue<int>("projectRecovery.secondsBetween") .get_value_or(0); double dblValue = ConfigService::Instance() - .getValue<double>("algorithms.retained") + .getValue<double>("projectRecovery.secondsBetween") .get_value_or(0); - TS_ASSERT_EQUALS(value, 50); - TS_ASSERT_EQUALS(dblValue, 50.0); + TS_ASSERT_EQUALS(value, 60); + TS_ASSERT_EQUALS(dblValue, 60.0); } void TestMissingProperty() { diff --git a/Framework/Properties/Mantid.properties.template b/Framework/Properties/Mantid.properties.template index aa98016f34e54182ce72ffdff53cd35db52ca658..028a3ecc5a97239449801f6056cc479da7d51882 100644 --- a/Framework/Properties/Mantid.properties.template +++ b/Framework/Properties/Mantid.properties.template @@ -111,9 +111,6 @@ icatDownload.directory = # ICat mount point. Directory where archive is mounted. See Facility.xml filelocation. icatDownload.mountPoint = -# The Number of algorithms properties to retain im memory for refence in scripts. -algorithms.retained = 50 - # Defines the maximum number of cores to use for OpenMP # For machine default set to 0 MultiThreaded.MaxCores = 0 diff --git a/Framework/PythonInterface/mantid/api/src/Exports/AlgorithmManager.cpp b/Framework/PythonInterface/mantid/api/src/Exports/AlgorithmManager.cpp index f851ae4a710f04aa39cb8b84c5c8474cbd03fc09..8379c24ee2b27b440da24dd697a62c41d7887d29 100644 --- a/Framework/PythonInterface/mantid/api/src/Exports/AlgorithmManager.cpp +++ b/Framework/PythonInterface/mantid/api/src/Exports/AlgorithmManager.cpp @@ -109,9 +109,6 @@ void export_AlgorithmManager() { "Creates an unmanaged algorithm.")) .def("size", &AlgorithmManagerImpl::size, arg("self"), "Returns the number of managed algorithms") - .def("setMaxAlgorithms", &AlgorithmManagerImpl::setMaxAlgorithms, - (arg("self"), arg("n")), - "Set the maximum number of allowed managed algorithms") .def("getAlgorithm", &getAlgorithm, (arg("self"), arg("id_holder")), "Return the algorithm instance identified by the given id.") .def("removeById", &removeById, (arg("self"), arg("id_holder")), diff --git a/Framework/PythonInterface/test/python/mantid/api/AlgorithmManagerTest.py b/Framework/PythonInterface/test/python/mantid/api/AlgorithmManagerTest.py index 9fa5ca60cbcb86f2e1f49fae6ec40ec803af1ba6..f78decc2c57eb71e11bf424a0e2bf4187f78bcdb 100644 --- a/Framework/PythonInterface/test/python/mantid/api/AlgorithmManagerTest.py +++ b/Framework/PythonInterface/test/python/mantid/api/AlgorithmManagerTest.py @@ -44,10 +44,11 @@ class AlgorithmManagerTest(unittest.TestCase): self.assertTrue(isinstance(alg, Algorithm)) def test_size_reports_number_of_managed_algorithms(self): - old_size = AlgorithmManager.size() - new_alg = AlgorithmManager.create("ConvertUnits") - new_size = AlgorithmManager.size() - self.assertEqual(new_size, old_size + 1) + # no longer deterministically possible to have a correct answer for size + # if test are run multi threaded + # just check we got an integer back + size = AlgorithmManager.size() + self.assertTrue(isinstance(size, int)) def test_getAlgorithm_returns_correct_instance(self): returned_instance = AlgorithmManager.create("ConvertUnits") diff --git a/Framework/PythonInterface/test/python/mantid/kernel/ConfigServiceTest.py b/Framework/PythonInterface/test/python/mantid/kernel/ConfigServiceTest.py index fbeb78d81a9979656937f3dfc2ce47249d629dc5..a8cfbaa84b7627f1deba821f6eac53a8e8f65db1 100644 --- a/Framework/PythonInterface/test/python/mantid/kernel/ConfigServiceTest.py +++ b/Framework/PythonInterface/test/python/mantid/kernel/ConfigServiceTest.py @@ -65,7 +65,7 @@ class ConfigServiceTest(unittest.TestCase): self.assertRaises(RuntimeError, config.getInstrument, "MadeUpInstrument") def test_service_acts_like_dictionary(self): - test_prop = "algorithms.retained" + test_prop = "projectRecovery.secondsBetween" self.assertTrue(config.hasProperty(test_prop)) dictcall = config[test_prop] fncall = config.getString(test_prop) diff --git a/Framework/PythonInterface/test/python/plugins/algorithms/WorkflowAlgorithms/SimpleShapeMonteCarloAbsorptionTest.py b/Framework/PythonInterface/test/python/plugins/algorithms/WorkflowAlgorithms/SimpleShapeMonteCarloAbsorptionTest.py index 7d427b698e7e08611e12496054c2bea8161ac1a6..8ab032e980d519f1fdfa8d48fcaf0d7e07baabac 100644 --- a/Framework/PythonInterface/test/python/plugins/algorithms/WorkflowAlgorithms/SimpleShapeMonteCarloAbsorptionTest.py +++ b/Framework/PythonInterface/test/python/plugins/algorithms/WorkflowAlgorithms/SimpleShapeMonteCarloAbsorptionTest.py @@ -13,7 +13,8 @@ import unittest class SimpleShapeMonteCarloAbsorptionTest(unittest.TestCase): - def setUp(self): + @classmethod + def setUpClass(self): red_ws = Load('irs26176_graphite002_red.nxs') red_ws = ConvertUnits( InputWorkspace=red_ws, @@ -38,7 +39,7 @@ class SimpleShapeMonteCarloAbsorptionTest(unittest.TestCase): 'OuterRadius': 2.0 }) - corrected = SimpleShapeMonteCarloAbsorption(InputWorkspace=self._red_ws, + __corrected_flat_plate = SimpleShapeMonteCarloAbsorption(InputWorkspace=self._red_ws, Shape='FlatPlate', Width=2.0, Thickness=2.0, @@ -46,7 +47,12 @@ class SimpleShapeMonteCarloAbsorptionTest(unittest.TestCase): # store the basic flat plate workspace so it can be compared with # others - self._corrected_flat_plate = corrected + self._corrected_flat_plate = __corrected_flat_plate + + @classmethod + def tearDownClass(self): + DeleteWorkspace(self._red_ws) + DeleteWorkspace(self._corrected_flat_plate) def _test_corrections_workspace(self, corr_ws): x_unit = corr_ws.getAxis(0).getUnit().unitID() @@ -61,10 +67,6 @@ class SimpleShapeMonteCarloAbsorptionTest(unittest.TestCase): blocksize = corr_ws.blocksize() self.assertEqual(blocksize, 1905) - def tearDown(self): - DeleteWorkspace(self._red_ws) - DeleteWorkspace(self._corrected_flat_plate) - def test_flat_plate(self): # Test flat plate shape diff --git a/docs/source/concepts/PropertiesFile.rst b/docs/source/concepts/PropertiesFile.rst index d6cf42272ef18eb1e93298931615262e1128613f..41994c91022d556c1d320c357dcd88db00c22817 100644 --- a/docs/source/concepts/PropertiesFile.rst +++ b/docs/source/concepts/PropertiesFile.rst @@ -36,9 +36,6 @@ General properties | ``algorithms.categories.hidden`` | A comma separated list of any categories of | ``Muons,Testing`` | | | algorithms that should be hidden in Mantid. | | +----------------------------------+--------------------------------------------------+------------------------+ -| ``algorithms.retained`` | The Number of algorithms properties to retain in | ``50`` | -| | memory for reference in scripts. | | -+----------------------------------+--------------------------------------------------+------------------------+ | ``curvefitting.guiExclude`` | A semicolon separated list of function names | ``ExpDecay;Gaussian;`` | | | that should be hidden in Mantid. | | +----------------------------------+--------------------------------------------------+------------------------+ diff --git a/scripts/Inelastic/vesuvio/commands.py b/scripts/Inelastic/vesuvio/commands.py index 9c80a35556b6918f8834f31529c4d9a3dde94751..5df034bc44150a9e199b3e3b0e5e9cda239675a9 100644 --- a/scripts/Inelastic/vesuvio/commands.py +++ b/scripts/Inelastic/vesuvio/commands.py @@ -198,10 +198,8 @@ class VesuvioTOFFitRoutineIteration(object): # Calculate corrections corrections_result = self._corrections(vesuvio_input.sample_data, vesuvio_input.container_data, index, all_mass_values, all_profiles, prefit_result[1], verbose_output) - # Calculate final fit fit_result = self._final_fit(corrections_result[-1], fit_mass_values, fit_profiles) - # Update output with results from fit _update_output(vesuvio_output, prefit_result, corrections_result, fit_result) @@ -210,7 +208,6 @@ class VesuvioTOFFitRoutineIteration(object): UnGroupWorkspace(corrections_result[0]) UnGroupWorkspace(corrections_result[1]) mtd.remove(prefit_result[1].name()) - mtd.remove(corrections_result[-1].name()) mtd.remove(fit_result[1].name()) return vesuvio_output