From 7568b001662ef0f7d60e7c091222ef61ad162b14 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Thu, 9 Jan 2025 14:02:53 +0000 Subject: [PATCH 01/19] Drop __init__ from AbinsAlgorithm It doesn't seem to do anything useful --- scripts/abins/abinsalgorithm.py | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/scripts/abins/abinsalgorithm.py b/scripts/abins/abinsalgorithm.py index 2f0112eaaf2a..40da1b1beb87 100644 --- a/scripts/abins/abinsalgorithm.py +++ b/scripts/abins/abinsalgorithm.py @@ -35,29 +35,6 @@ class AbinsAlgorithm: """Class providing shared utility for multiple inheritence by 1D, 2D implementations""" - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) # i.e. forward everything to PythonAlgorithm - - # User input private properties - self._instrument_name = None - - self._vibrational_or_phonon_data_file = None - self._ab_initio_program = None - self._out_ws_name = None - self._temperature = None - self._atoms = None - self._sum_contributions = None - self._save_ascii = None - self._scale_by_cross_section = None - - self._num_quantum_order_events = None - self._autoconvolution = None - self._energy_units = None - - # Interally-used private properties - self._max_event_order = None - self._bin_width = None - def get_common_properties(self) -> None: """From user input, set properties common to Abins 1D and 2D versions""" self._ab_initio_program = self.getProperty("AbInitioProgram").value From ce8ba6c4cf13531f7aa0c43aa082979e10d55f53 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Thu, 9 Jan 2025 14:37:30 +0000 Subject: [PATCH 02/19] Accept CacheDirectory user input to Abins (currently does nothing) Add an input parameter "CacheDirectory" which is validated as being writeable. This gives a nice user experience, with default value appearing when the field is emptied. The actual caching mechanics still need to be connected to this parameter. --- scripts/abins/abinsalgorithm.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/scripts/abins/abinsalgorithm.py b/scripts/abins/abinsalgorithm.py index 40da1b1beb87..1541991cbbea 100644 --- a/scripts/abins/abinsalgorithm.py +++ b/scripts/abins/abinsalgorithm.py @@ -11,6 +11,7 @@ import os from pathlib import Path import re +from tempfile import NamedTemporaryFile from typing import Dict, Iterable, List, Literal, Tuple, Union import yaml @@ -22,7 +23,7 @@ import numpy as np from mantid.api import mtd, FileAction, FileProperty, WorkspaceGroup, WorkspaceProperty -from mantid.kernel import Direction, StringListValidator, StringArrayProperty, logger +from mantid.kernel import ConfigService, Direction, StringListValidator, StringArrayProperty, logger from mantid.simpleapi import CloneWorkspace, SaveAscii, Scale from abins.atominfo import AtomInfo @@ -50,6 +51,8 @@ def get_common_properties(self) -> None: self._energy_units = self.getProperty("EnergyUnits").value + self._cache_directory = self.getProperty("CacheDirectory").value + # conversion from str to int self._num_quantum_order_events = int(self.getProperty("QuantumOrderEventsNumber").value) self._max_event_order = self._num_quantum_order_events # This default can be replaced in child class @@ -85,6 +88,17 @@ def declare_common_properties(self, version: int = 1) -> None: self.declareProperty(WorkspaceProperty("OutputWorkspace", "", Direction.Output), doc="Name to give the output workspace.") + self.declareProperty( + FileProperty( + "CacheDirectory", + defaultValue=ConfigService.getString("defaultsave.directory"), + action=FileAction.Directory, + direction=Direction.Input, + extensions=[""], + ), + doc="Directory in which Abins reads/writes cache files", + ) + self.declareProperty( name="TemperatureInKelvin", direction=Direction.Input, @@ -241,6 +255,13 @@ def validate_common_inputs(self, issues: dict = None) -> Dict[str, str]: issues["OutputWorkspace"] = "Keyword: " + word + " cannot be used in the name of workspace." break + cache_directory = Path(self.getProperty("CacheDirectory").value) + try: + with NamedTemporaryFile(mode="w", dir=cache_directory) as fd: + print("check this directory is writeable", file=fd) + except IOError: + issues["CacheDirectory"] = "Cache directory is not writeable" + temperature = self.getProperty("TemperatureInKelvin").value if temperature < 0: issues["TemperatureInKelvin"] = "Temperature must be positive." From 257d368cda2b44c9a863b7dcb7c6ee8244ae4117 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Fri, 10 Jan 2025 10:51:40 +0000 Subject: [PATCH 03/19] Modify Abins test cleanup to take cache dir; update loader tests Other tests (which create cache in different ways) still fail, this is expected. --- scripts/abins/abinsdata.py | 4 +++- scripts/abins/io.py | 18 ++++++++--------- scripts/abins/test_helpers.py | 20 ++++--------------- .../test/Abins/AbinsCalculateSPowderTest.py | 6 +++++- scripts/test/Abins/AbinsIOmoduleTest.py | 4 +++- scripts/test/Abins/AbinsLoadCASTEPTest.py | 4 +++- scripts/test/Abins/AbinsLoadCRYSTALTest.py | 4 +++- scripts/test/Abins/AbinsLoadDMOL3Test.py | 4 +++- scripts/test/Abins/AbinsLoadGAUSSIANTest.py | 4 +++- scripts/test/Abins/AbinsLoadJSONTest.py | 5 ++++- scripts/test/Abins/AbinsLoadPhonopyTest.py | 5 ++++- scripts/test/Abins/AbinsLoadVASPTest.py | 6 ++++-- 12 files changed, 48 insertions(+), 36 deletions(-) diff --git a/scripts/abins/abinsdata.py b/scripts/abins/abinsdata.py index 9063cd9559c8..94f22496d177 100644 --- a/scripts/abins/abinsdata.py +++ b/scripts/abins/abinsdata.py @@ -4,6 +4,7 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + +from pathlib import Path from typing import Any, Dict, Type, TypedDict, TypeVar from pydantic import validate_call @@ -32,12 +33,13 @@ def __init__(self, *, k_points_data: KpointsData, atoms_data: AtomsData) -> None self._check_consistent_dimensions() @staticmethod - def from_calculation_data(filename: str, ab_initio_program: str) -> "AbinsData": + def from_calculation_data(filename: str, ab_initio_program: str, cache_directory: Path) -> "AbinsData": """ Get AbinsData from ab initio calculation output file. :param filename: Path to vibration/phonon data file :param ab_initio_program: Program which generated data file; this should be a key in AbinsData.ab_initio_loaders + :param cache_directory: Location for cache files """ from abins.input import all_loaders # Defer import to avoid loops when abins.__init__ imports AbinsData diff --git a/scripts/abins/io.py b/scripts/abins/io.py index 983041f88bcb..bdd82efd147c 100644 --- a/scripts/abins/io.py +++ b/scripts/abins/io.py @@ -8,6 +8,7 @@ import io import json import os +from pathlib import Path import subprocess import shutil from typing import List, Optional @@ -34,6 +35,7 @@ class IO(BaseModel): setting: str = "" autoconvolution: int = 10 temperature: float = None + cache_directory: Path = None @staticmethod def _dir_is_not_writeable(directory: str): @@ -60,6 +62,9 @@ def model_post_init(self, __context): except ValueError as err: logger.error(str(err)) + if self.cache_directory is None: + self.cache_directory = Path(ConfigService.getString("defaultsave.directory")) + # extract name of file from the full path in the platform independent way filename = os.path.basename(self.input_filename) @@ -68,13 +73,12 @@ def model_post_init(self, __context): else: core_name = filename # e.g. OUTCAR -> OUTCAR (core_name) -> OUTCAR.hdf5 - if self._dir_is_not_writeable(self.get_save_dir_path()): + if self._dir_is_not_writeable(str(self.cache_directory)): raise Exception( - "Could not write a file to the default save directory: " - "this is required for caching. Please set 'Default Save Directory' to " - "a writeable location using the File/Manage User Directories interface." + f"Could not write a file to the cache directory {self.cache_directory}. " + "Please check this location is reasonable." ) - self._hdf_filename = os.path.join(self.get_save_dir_path(), core_name + ".hdf5") # name of hdf file + self._hdf_filename = str(self.cache_directory / f"{core_name}.hdf5") self._attributes = {} # attributes for group @@ -85,10 +89,6 @@ def model_post_init(self, __context): # Fields which have a form of empty dictionaries have to be set by an inheriting class. - @staticmethod - def get_save_dir_path() -> str: - return ConfigService.getString("defaultsave.directory") - def _valid_hash(self): """ Checks if input ab initio file and content of HDF file are consistent. diff --git a/scripts/abins/test_helpers.py b/scripts/abins/test_helpers.py index f7c6a543dae8..f64bb72b73e6 100644 --- a/scripts/abins/test_helpers.py +++ b/scripts/abins/test_helpers.py @@ -15,7 +15,6 @@ from abins.atomsdata import _AtomData from abins.kpointsdata import KpointData -import abins.io # Module with helper functions used to create tests. @@ -41,26 +40,15 @@ def find_file(filename: str, try_upcase_suffix: bool = True) -> str: @validate_call -def remove_output_files(list_of_names: List[str]) -> None: +def remove_output_files(list_of_names: List[str], directory: Path) -> None: """Removes output files created during a test.""" - - # import ConfigService here to avoid: - # RuntimeError: Pickling of "mantid.kernel._kernel.ConfigServiceImpl" - # instances is not enabled (http://www.boost.org/libs/python/doc/v2/pickle.html) - - from mantid.kernel import ConfigService - - save_dir_path = ConfigService.getString("defaultsave.directory") - if save_dir_path != "": # default save directory set - all_files = os.listdir(save_dir_path) - else: - all_files = os.listdir(os.getcwd()) + all_files = os.listdir(directory) for filename in all_files: for name in list_of_names: if name in filename: - full_path = os.path.join(abins.io.IO.get_save_dir_path(), filename) - if os.path.isfile(full_path): + full_path = directory / filename + if full_path.exists(): os.remove(full_path) break diff --git a/scripts/test/Abins/AbinsCalculateSPowderTest.py b/scripts/test/Abins/AbinsCalculateSPowderTest.py index 146255ba2daf..4ca0a2821918 100644 --- a/scripts/test/Abins/AbinsCalculateSPowderTest.py +++ b/scripts/test/Abins/AbinsCalculateSPowderTest.py @@ -42,7 +42,11 @@ def setUp(self): self._instruments_defaults = deepcopy(abins.parameters.instruments) def tearDown(self): - abins.test_helpers.remove_output_files(list_of_names=["_CalculateSPowder"]) + from mantid.kernel import ConfigService + + abins.test_helpers.remove_output_files( + list_of_names=["_CalculateSPowder"], directory=ConfigService.getString("defaultsave.directory") + ) abins.parameters.performance["threads"] = self.default_threads abins.parameters.instruments.update(self._instruments_defaults) diff --git a/scripts/test/Abins/AbinsIOmoduleTest.py b/scripts/test/Abins/AbinsIOmoduleTest.py index 36bb60b5a43d..29cc0cb0de3f 100644 --- a/scripts/test/Abins/AbinsIOmoduleTest.py +++ b/scripts/test/Abins/AbinsIOmoduleTest.py @@ -13,7 +13,9 @@ class IOTest(unittest.TestCase): def tearDown(self): - test_helpers.remove_output_files(list_of_names=["Cars", "temphgfrt"]) + from mantid.kernel import ConfigService + + test_helpers.remove_output_files(list_of_names=["Cars", "temphgfrt"], directory=ConfigService.getString("defaultsave.directory")) @staticmethod def _save_stuff(): diff --git a/scripts/test/Abins/AbinsLoadCASTEPTest.py b/scripts/test/Abins/AbinsLoadCASTEPTest.py index 175ee1aa7138..5424778ba886 100644 --- a/scripts/test/Abins/AbinsLoadCASTEPTest.py +++ b/scripts/test/Abins/AbinsLoadCASTEPTest.py @@ -45,7 +45,9 @@ def test_non_existing_file(self): CASTEPLoader(input_ab_initio_filename=1) def tearDown(self): - abins.test_helpers.remove_output_files(list_of_names=["_LoadCASTEP"]) + from mantid.kernel import ConfigService + + abins.test_helpers.remove_output_files(list_of_names=["_LoadCASTEP"], directory=ConfigService.getString("defaultsave.directory")) # *************************** USE CASES ******************************************** # =================================================================================== diff --git a/scripts/test/Abins/AbinsLoadCRYSTALTest.py b/scripts/test/Abins/AbinsLoadCRYSTALTest.py index 8cd5ff7519c5..7fbd76678e66 100644 --- a/scripts/test/Abins/AbinsLoadCRYSTALTest.py +++ b/scripts/test/Abins/AbinsLoadCRYSTALTest.py @@ -12,7 +12,9 @@ class AbinsLoadCRYSTALTest(unittest.TestCase, abins.input.Tester): def tearDown(self): - abins.test_helpers.remove_output_files(list_of_names=["_LoadCRYSTAL"]) + from mantid.kernel import ConfigService + + abins.test_helpers.remove_output_files(list_of_names=["_LoadCRYSTAL"], directory=ConfigService.getString("defaultsave.directory")) # *************************** USE CASES ********************************************* # =================================================================================== diff --git a/scripts/test/Abins/AbinsLoadDMOL3Test.py b/scripts/test/Abins/AbinsLoadDMOL3Test.py index cc31dde4d211..981f13758e44 100644 --- a/scripts/test/Abins/AbinsLoadDMOL3Test.py +++ b/scripts/test/Abins/AbinsLoadDMOL3Test.py @@ -15,7 +15,9 @@ class AbinsLoadDMOL3Test(unittest.TestCase, abins.input.Tester): """Check entire input files against expected outputs""" def tearDown(self): - abins.test_helpers.remove_output_files(list_of_names=["_LoadDMOL3"]) + from mantid.kernel import ConfigService + + abins.test_helpers.remove_output_files(list_of_names=["_LoadDMOL3"], directory=ConfigService.getString("defaultsave.directory")) _gamma_dmol3 = "LTA_40_O2_LoadDMOL3" _gamma_no_h_dmol3 = "Na2SiF6_LoadDMOL3" diff --git a/scripts/test/Abins/AbinsLoadGAUSSIANTest.py b/scripts/test/Abins/AbinsLoadGAUSSIANTest.py index 8a0660c9b13c..f955b87dddc9 100644 --- a/scripts/test/Abins/AbinsLoadGAUSSIANTest.py +++ b/scripts/test/Abins/AbinsLoadGAUSSIANTest.py @@ -12,7 +12,9 @@ class AbinsLoadGAUSSIANTest(unittest.TestCase, abins.input.Tester): def tearDown(self): - abins.test_helpers.remove_output_files(list_of_names=["_LoadGAUSSIAN"]) + from mantid.kernel import ConfigService + + abins.test_helpers.remove_output_files(list_of_names=["_LoadGAUSSIAN"], directory=ConfigService.getString("defaultsave.directory")) # *************************** USE CASES ******************************************** diff --git a/scripts/test/Abins/AbinsLoadJSONTest.py b/scripts/test/Abins/AbinsLoadJSONTest.py index 6ee6b519e3d5..98eeff503594 100644 --- a/scripts/test/Abins/AbinsLoadJSONTest.py +++ b/scripts/test/Abins/AbinsLoadJSONTest.py @@ -17,7 +17,10 @@ def setUp(self): abins.parameters.sampling["force_constants"]["qpt_cutoff"] = 4.0 def tearDown(self): - abins.test_helpers.remove_output_files(list_of_names=["_LoadJSON"]) + from mantid.kernel import ConfigService + + abins.test_helpers.remove_output_files(list_of_names=["_LoadJSON"], directory=ConfigService.getString("defaultsave.directory")) + abins.parameters.sampling["force_constants"]["qpt_cutoff"] = self.default_cutoff def test_json_1(self): diff --git a/scripts/test/Abins/AbinsLoadPhonopyTest.py b/scripts/test/Abins/AbinsLoadPhonopyTest.py index dd1feb591276..11a0f2f315ab 100644 --- a/scripts/test/Abins/AbinsLoadPhonopyTest.py +++ b/scripts/test/Abins/AbinsLoadPhonopyTest.py @@ -21,7 +21,10 @@ def setUp(self): abins.parameters.sampling["force_constants"]["qpt_cutoff"] = 4.0 def tearDown(self): - abins.test_helpers.remove_output_files(list_of_names=["_LoadPhonopy"]) + from mantid.kernel import ConfigService + + abins.test_helpers.remove_output_files(list_of_names=["_LoadPhonopy"], directory=ConfigService.getString("defaultsave.directory")) + abins.parameters.sampling["force_constants"]["qpt_cutoff"] = self.default_cutoff def test_non_existing_file(self): diff --git a/scripts/test/Abins/AbinsLoadVASPTest.py b/scripts/test/Abins/AbinsLoadVASPTest.py index 8c2324cb0eb5..932d01fa55e7 100644 --- a/scripts/test/Abins/AbinsLoadVASPTest.py +++ b/scripts/test/Abins/AbinsLoadVASPTest.py @@ -12,8 +12,10 @@ class AbinsLoadVASPTest(unittest.TestCase, abins.input.Tester): def tearDown(self): - # Remove ref files from .check() calls - abins.test_helpers.remove_output_files(list_of_names=["_LoadVASP"]) + # Remove cache files + from mantid.kernel import ConfigService + + abins.test_helpers.remove_output_files(list_of_names=["_LoadVASP"], directory=ConfigService.getString("defaultsave.directory")) def test_non_existing_file(self): with self.assertRaises(IOError): From 306f7d7af6aa7341d7ad6a1b3c6fb8c5e13be837 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Mon, 13 Jan 2025 13:32:31 +0000 Subject: [PATCH 04/19] Update more tests for explicit cache dir --- scripts/test/Abins/AbinsDataTest.py | 14 ++++++++++++-- scripts/test/Abins/AbinsPowderCalculatorTest.py | 6 +++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/scripts/test/Abins/AbinsDataTest.py b/scripts/test/Abins/AbinsDataTest.py index 5acf42a822f5..cc402dc3140d 100644 --- a/scripts/test/Abins/AbinsDataTest.py +++ b/scripts/test/Abins/AbinsDataTest.py @@ -22,6 +22,10 @@ class TestAbinsData(unittest.TestCase): def setUp(self): + from mantid.kernel import ConfigService + + self.cache_directory = ConfigService.getString("defaultsave.directory") + self.mock_ad = MagicMock(spec=abins.AtomsData) self.mock_kpd = MagicMock(spec=abins.KpointsData) @@ -34,7 +38,9 @@ def test_init_typeerror(self): def test_init_noloader(self): with self.assertRaises(ValueError): AbinsData.from_calculation_data( - abins.test_helpers.find_file("squaricn_sum_LoadCASTEP.phonon"), ab_initio_program="fake_program" + abins.test_helpers.find_file("squaricn_sum_LoadCASTEP.phonon"), + ab_initio_program="fake_program", + cache_directory=self.cache_directory, ) def test_data_content(self): @@ -54,13 +60,17 @@ def get_formatted_data(self): class TestAbinsDataFromCalculation(unittest.TestCase): def setUp(self): + from mantid.kernel import ConfigService + + self.cache_directory = ConfigService.getString("defaultsave.directory") + all_loaders["DUMMYLOADER"] = DummyLoader def tearDown(self): del all_loaders["DUMMYLOADER"] def test_with_dummy_loader(self): - data = AbinsData.from_calculation_data("dummy_file.ext", "DummyLoader") + data = AbinsData.from_calculation_data("dummy_file.ext", "DummyLoader", cache_directory=self.cache_directory) self.assertEqual(data, "FORMATTED DATA") diff --git a/scripts/test/Abins/AbinsPowderCalculatorTest.py b/scripts/test/Abins/AbinsPowderCalculatorTest.py index 77828c842a84..ea340fe1fa33 100644 --- a/scripts/test/Abins/AbinsPowderCalculatorTest.py +++ b/scripts/test/Abins/AbinsPowderCalculatorTest.py @@ -23,7 +23,11 @@ class PowderCalculatorTest(unittest.TestCase): # test input def tearDown(self): - abins.test_helpers.remove_output_files(list_of_names=["CalculatePowder"]) + from mantid.kernel import ConfigService + + abins.test_helpers.remove_output_files( + list_of_names=["CalculatePowder"], directory=ConfigService.getString("defaultsave.directory") + ) def test_wrong_input(self): full_path_filename = abins.test_helpers.find_file(filename=self._si2 + ".json") From 9a2df85dd094130e0fc94a8c2d56aa1a4caeb96c Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Wed, 15 Jan 2025 09:51:55 +0000 Subject: [PATCH 05/19] Fix Abins Unit tests; revised cleanup function signature We are also starting to pass the cache directory to Abins explicitly, although it is not used yet. --- .../plugins/algorithms/Abins.py | 4 ++- .../plugins/algorithms/Abins2BasicTest.py | 6 ++-- .../algorithms/AbinsAdvancedParametersTest.py | 28 +++++++++++++++-- .../plugins/algorithms/AbinsBasicTest.py | 30 ++++++++++++++++++- scripts/abins/abins2.py | 4 ++- scripts/abins/io.py | 14 +++++++-- 6 files changed, 76 insertions(+), 10 deletions(-) diff --git a/Framework/PythonInterface/plugins/algorithms/Abins.py b/Framework/PythonInterface/plugins/algorithms/Abins.py index f8a5e3e89e70..d72a2ed4746f 100644 --- a/Framework/PythonInterface/plugins/algorithms/Abins.py +++ b/Framework/PythonInterface/plugins/algorithms/Abins.py @@ -108,7 +108,9 @@ def PyExec(self): prog_reporter.report("Input data from the user has been collected.") # 2) read ab initio data - ab_initio_data = abins.AbinsData.from_calculation_data(self._vibrational_or_phonon_data_file, self._ab_initio_program) + ab_initio_data = abins.AbinsData.from_calculation_data( + self._vibrational_or_phonon_data_file, self._ab_initio_program, cache_directory=self._cache_directory + ) prog_reporter.report("Vibrational/phonon data has been read.") # 3) calculate S diff --git a/Framework/PythonInterface/test/python/plugins/algorithms/Abins2BasicTest.py b/Framework/PythonInterface/test/python/plugins/algorithms/Abins2BasicTest.py index d7dab3c3b4e1..c2eb2898204d 100644 --- a/Framework/PythonInterface/test/python/plugins/algorithms/Abins2BasicTest.py +++ b/Framework/PythonInterface/test/python/plugins/algorithms/Abins2BasicTest.py @@ -5,6 +5,7 @@ # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + import logging +from pathlib import Path import tempfile import unittest from unittest.mock import patch @@ -40,7 +41,7 @@ class AbinsBasicTest(unittest.TestCase): _tmp_cache_dir = None def get_cache_dir(self): - return self._tmp_cache_dir.name + return Path(self._tmp_cache_dir.name) @classmethod def setUpClass(cls): @@ -63,7 +64,8 @@ def tearDown(self): "benzene_exp", "benzene_Abins", "numbered", - ] + ], + directory=self.get_cache_dir(), ) mtd.clear() diff --git a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py index a3e3dc5178cd..c06a6db14fa7 100644 --- a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py +++ b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py @@ -4,8 +4,10 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + +from pathlib import Path import unittest from mantid.simpleapi import Abins, mtd +from mantid.kernel import ConfigService from abins import test_helpers import abins.parameters @@ -23,6 +25,7 @@ def setUp(self): # set up input for Abins self._Si2 = "Si2-sc_AbinsAdvancedParameters" self._wrk_name = self._Si2 + "_ref" + self._cache_directory = ConfigService.getString("defaultsave.directory") # before each test set abins.parameters to default values abins.parameters.instruments = { @@ -53,7 +56,7 @@ def setUp(self): abins.parameters.performance = {"optimal_size": int(5e6), "threads": 1} def tearDown(self): - test_helpers.remove_output_files(list_of_names=["_AbinsAdvanced"]) + test_helpers.remove_output_files(list_of_names=["_AbinsAdvanced"], directory=Path(self._cache_directory)) mtd.clear() def test_wrong_fwhm(self): @@ -83,6 +86,7 @@ def test_wrong_tosca_final_energy(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_tosca_cos_scattering_angle(self): @@ -99,6 +103,7 @@ def test_wrong_tosca_cos_scattering_angle(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_tosca_resolution_constant_A(self): @@ -110,6 +115,7 @@ def test_wrong_tosca_resolution_constant_A(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_tosca_resolution_constant_B(self): @@ -120,6 +126,7 @@ def test_wrong_tosca_resolution_constant_B(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_tosca_resolution_constant_C(self): @@ -130,6 +137,7 @@ def test_wrong_tosca_resolution_constant_C(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) # tests for folders @@ -146,6 +154,7 @@ def test_wrong_dft_group(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_powder_data_group(self): @@ -161,6 +170,7 @@ def test_wrong_powder_data_group(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_crystal_data_group(self): @@ -176,6 +186,7 @@ def test_wrong_crystal_data_group(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_powder_s_data_group(self): @@ -191,6 +202,7 @@ def test_wrong_powder_s_data_group(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_doubled_name(self): @@ -203,6 +215,7 @@ def test_doubled_name(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_min_wavenumber(self): @@ -218,6 +231,7 @@ def test_wrong_min_wavenumber(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_max_wavenumber(self): @@ -233,6 +247,7 @@ def test_wrong_max_wavenumber(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_energy_window(self): @@ -245,6 +260,7 @@ def test_wrong_energy_window(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_s_absolute_threshold(self): @@ -258,6 +274,7 @@ def test_wrong_s_absolute_threshold(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_s_relative_threshold(self): @@ -271,6 +288,7 @@ def test_wrong_s_relative_threshold(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_optimal_size(self): @@ -286,6 +304,7 @@ def test_wrong_optimal_size(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_wrong_threads(self): @@ -297,11 +316,16 @@ def test_wrong_threads(self): Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) def test_good_case(self): good_names = [self._wrk_name, self._wrk_name + "_Si", self._wrk_name + "_Si_total"] - Abins(VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name) + Abins( + VibrationalOrPhononFile=self._Si2 + ".phonon", + OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, + ) names = mtd.getObjectNames() # Builtin cmp has been removed in Python 3 diff --git a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py index 59fc91f7716d..e81d0f1087f6 100644 --- a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py +++ b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py @@ -30,6 +30,10 @@ class AbinsBasicTest(unittest.TestCase): _workspace_name = "output_workspace" _tolerance = 0.0001 + from mantid.kernel import ConfigService + + _cache_directory = ConfigService.getString("defaultsave.directory") + def tearDown(self): abins.test_helpers.remove_output_files( list_of_names=[ @@ -42,7 +46,8 @@ def tearDown(self): "benzene_Abins", "experimental", "numbered", - ] + ], + directory=self._cache_directory, ) mtd.clear() @@ -56,6 +61,7 @@ def test_wrong_input(self): Abins, VibrationalOrPhononFile="Si2-sc_wrong.phonon", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) # wrong extension of phonon file in case of CASTEP @@ -65,6 +71,7 @@ def test_wrong_input(self): Abins, VibrationalOrPhononFile="Si2-sc.wrong_phonon", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) # wrong extension of phonon file in case of CRYSTAL @@ -75,6 +82,7 @@ def test_wrong_input(self): AbInitioProgram="CRYSTAL", VibrationalOrPhononFile="MgO.wrong_out", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) # no name for workspace @@ -88,6 +96,7 @@ def test_wrong_input(self): VibrationalOrPhononFile=self._si2 + ".phonon", TemperatureInKelvin=self._temperature, OutputWorkspace=self._workspace_name + "total", + CacheDirectory=self._cache_directory, ) # negative temperature in K @@ -98,6 +107,7 @@ def test_wrong_input(self): VibrationalOrPhononFile=self._si2 + ".phonon", TemperatureInKelvin=-1.0, OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) # negative scale @@ -108,6 +118,7 @@ def test_wrong_input(self): VibrationalOrPhononFile=self._si2 + ".phonon", Scale=-0.2, OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) # unknown instrument @@ -118,6 +129,7 @@ def test_wrong_input(self): VibrationalOrPhononFile=self._si2 + ".phonon", Instrument="UnknownInstrument", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) # test if intermediate results are consistent @@ -132,6 +144,7 @@ def test_non_unique_elements(self): VibrationalOrPhononFile=self._squaricn + ".phonon", Atoms="C,C,H", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) def test_non_unique_atoms(self): @@ -145,6 +158,7 @@ def test_non_unique_atoms(self): VibrationalOrPhononFile=self._squaricn + ".phonon", Atoms="atom_1,atom_2,atom1", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) def test_non_existing_atoms(self): @@ -159,6 +173,7 @@ def test_non_existing_atoms(self): VibrationalOrPhononFile=self._squaricn + ".phonon", Atoms="N", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) def test_atom_index_limits(self): @@ -172,6 +187,7 @@ def test_atom_index_limits(self): VibrationalOrPhononFile=self._squaricn + ".phonon", Atoms=ATOM_PREFIX + "0", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) self.assertRaisesRegex( RuntimeError, @@ -180,6 +196,7 @@ def test_atom_index_limits(self): VibrationalOrPhononFile=self._squaricn + ".phonon", Atoms=ATOM_PREFIX + "61", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) def test_atom_index_invalid(self): @@ -193,6 +210,7 @@ def test_atom_index_invalid(self): VibrationalOrPhononFile=self._squaricn + ".phonon", Atoms=ATOM_PREFIX + "-3", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) self.assertRaisesRegex( RuntimeError, @@ -201,6 +219,7 @@ def test_atom_index_invalid(self): VibrationalOrPhononFile=self._squaricn + ".phonon", Atoms=ATOM_PREFIX + "_#4", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) def test_scale(self): @@ -220,6 +239,7 @@ def test_scale(self): QuantumOrderEventsNumber=self._quantum_order_events_number, ScaleByCrossSection=self._cross_section_factor, OutputWorkspace=self._squaricn + "_ref", + CacheDirectory=self._cache_directory, ) wrk = Abins( @@ -234,6 +254,7 @@ def test_scale(self): Scale=10, ScaleByCrossSection=self._cross_section_factor, OutputWorkspace="squaricn_scale", + CacheDirectory=self._cache_directory, ) ref = Scale(wrk_ref, Factor=10) @@ -255,6 +276,7 @@ def test_lagrange_exists(self): QuantumOrderEventsNumber=self._quantum_order_events_number, ScaleByCrossSection=self._cross_section_factor, OutputWorkspace="squaricn-lagrange", + CacheDirectory=self._cache_directory, ) def test_exp(self): @@ -275,6 +297,7 @@ def test_exp(self): QuantumOrderEventsNumber=self._quantum_order_events_number, ScaleByCrossSection=self._cross_section_factor, OutputWorkspace="benzene_exp", + CacheDirectory=self._cache_directory, ) # load experimental data @@ -303,6 +326,7 @@ def test_partial_by_element(self): QuantumOrderEventsNumber=self._quantum_order_events_number, ScaleByCrossSection=self._cross_section_factor, OutputWorkspace=self._squaricn + "_ref", + CacheDirectory=self._cache_directory, ) wks_all_atoms_explicitly = Abins( @@ -311,6 +335,7 @@ def test_partial_by_element(self): SumContributions=self._sum_contributions, QuantumOrderEventsNumber=self._quantum_order_events_number, OutputWorkspace="explicit", + CacheDirectory=self._cache_directory, ) wks_all_atoms_default = Abins( @@ -318,6 +343,7 @@ def test_partial_by_element(self): SumContributions=self._sum_contributions, QuantumOrderEventsNumber=self._quantum_order_events_number, OutputWorkspace="default", + CacheDirectory=self._cache_directory, ) # Python 3 has no guarantee of dict order so the workspaces in the group may be in @@ -348,6 +374,7 @@ def test_partial_by_number(self): QuantumOrderEventsNumber=self._quantum_order_events_number, ScaleByCrossSection=self._cross_section_factor, OutputWorkspace=self._squaricn + "_ref", + CacheDirectory=self._cache_directory, ) numbered_workspace_name = "numbered" @@ -359,6 +386,7 @@ def test_partial_by_number(self): QuantumOrderEventsNumber=self._quantum_order_events_number, ScaleByCrossSection=self._cross_section_factor, OutputWorkspace=numbered_workspace_name, + CacheDirectory=self._cache_directory, ) wrk_ref_names = list(wrk_ref.getNames()) diff --git a/scripts/abins/abins2.py b/scripts/abins/abins2.py index e9616cc5617b..cc9a87f7dad9 100644 --- a/scripts/abins/abins2.py +++ b/scripts/abins/abins2.py @@ -86,7 +86,9 @@ def PyExec(self): prog_reporter.report("Input data from the user has been collected.") # 2) read ab initio data - ab_initio_data = abins.AbinsData.from_calculation_data(self._vibrational_or_phonon_data_file, self._ab_initio_program) + ab_initio_data = abins.AbinsData.from_calculation_data( + self._vibrational_or_phonon_data_file, self._ab_initio_program, cache_directory=self._cache_directory + ) prog_reporter.report("Vibrational/phonon data has been read.") # 3) calculate S diff --git a/scripts/abins/io.py b/scripts/abins/io.py index bdd82efd147c..280ea1c6dd4a 100644 --- a/scripts/abins/io.py +++ b/scripts/abins/io.py @@ -20,7 +20,7 @@ import abins from abins.constants import AB_INITIO_FILE_EXTENSIONS, BUF, HDF5_ATTR_TYPE -from mantid.kernel import logger, ConfigService +from mantid.kernel import logger class IO(BaseModel): @@ -54,6 +54,14 @@ def _dir_is_not_writeable(directory: str): return False + def get_save_dir_path(self): + from mantid.kernel import ConfigService + + if self.cache_directory: + return self.cache_directory + + return Path(ConfigService.getString("defaultsave.directory")) + def model_post_init(self, __context): try: self._hash_input_filename = self.calculate_ab_initio_file_hash() @@ -62,8 +70,8 @@ def model_post_init(self, __context): except ValueError as err: logger.error(str(err)) - if self.cache_directory is None: - self.cache_directory = Path(ConfigService.getString("defaultsave.directory")) + # Set default if necessary + self.cache_directory = self.get_save_dir_path() # extract name of file from the full path in the platform independent way filename = os.path.basename(self.input_filename) From 04581ca33ee2a985e3677d06739fa1b373be51b0 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Wed, 15 Jan 2025 14:59:14 +0000 Subject: [PATCH 06/19] Use explicit cache_directory variable in Abins algorithms --- .../PythonInterface/plugins/algorithms/Abins.py | 1 + .../PythonInterface/plugins/algorithms/Abins2D.py | 7 ++++++- scripts/abins/abins2.py | 1 + scripts/abins/abinsalgorithm.py | 2 +- scripts/abins/io.py | 2 +- scripts/abins/powdercalculator.py | 12 +++++++++--- scripts/abins/scalculatorfactory.py | 5 +++++ scripts/abins/spowdersemiempiricalcalculator.py | 7 ++++++- 8 files changed, 30 insertions(+), 7 deletions(-) diff --git a/Framework/PythonInterface/plugins/algorithms/Abins.py b/Framework/PythonInterface/plugins/algorithms/Abins.py index d72a2ed4746f..ade9953076f3 100644 --- a/Framework/PythonInterface/plugins/algorithms/Abins.py +++ b/Framework/PythonInterface/plugins/algorithms/Abins.py @@ -131,6 +131,7 @@ def PyExec(self): instrument=self._instrument, quantum_order_num=self._num_quantum_order_events, autoconvolution_max=autoconvolution_max, + cache_directory=self._cache_directory, ) s_calculator.progress_reporter = prog_reporter s_data = s_calculator.get_formatted_data() diff --git a/Framework/PythonInterface/plugins/algorithms/Abins2D.py b/Framework/PythonInterface/plugins/algorithms/Abins2D.py index 9157392ca380..d4f4979edd7f 100644 --- a/Framework/PythonInterface/plugins/algorithms/Abins2D.py +++ b/Framework/PythonInterface/plugins/algorithms/Abins2D.py @@ -118,7 +118,11 @@ def PyExec(self): prog_reporter.report("Input data from the user has been collected.") # 2) read ab initio data - ab_initio_data = abins.AbinsData.from_calculation_data(self._vibrational_or_phonon_data_file, self._ab_initio_program) + ab_initio_data = abins.AbinsData.from_calculation_data( + self._vibrational_or_phonon_data_file, + self._ab_initio_program, + cache_directory=self._cache_directory, + ) prog_reporter.report("Vibrational/phonon data has been read.") # 3) calculate S @@ -139,6 +143,7 @@ def PyExec(self): autoconvolution_max=autoconvolution_max, instrument=self._instrument, quantum_order_num=self._num_quantum_order_events, + cache_directory=self._cache_directory, ) s_calculator.progress_reporter = prog_reporter s_data = s_calculator.get_formatted_data() diff --git a/scripts/abins/abins2.py b/scripts/abins/abins2.py index cc9a87f7dad9..7e53e9d3847d 100644 --- a/scripts/abins/abins2.py +++ b/scripts/abins/abins2.py @@ -127,6 +127,7 @@ def PyExec(self): instrument=self._instrument, quantum_order_num=quantum_order_num, autoconvolution_max=autoconvolution_max, + cache_directory=self._cache_directory, ) s_calculator.progress_reporter = prog_reporter s_data = s_calculator.get_formatted_data() diff --git a/scripts/abins/abinsalgorithm.py b/scripts/abins/abinsalgorithm.py index 1541991cbbea..83c9087cc7b5 100644 --- a/scripts/abins/abinsalgorithm.py +++ b/scripts/abins/abinsalgorithm.py @@ -51,7 +51,7 @@ def get_common_properties(self) -> None: self._energy_units = self.getProperty("EnergyUnits").value - self._cache_directory = self.getProperty("CacheDirectory").value + self._cache_directory = Path(self.getProperty("CacheDirectory").value) # conversion from str to int self._num_quantum_order_events = int(self.getProperty("QuantumOrderEventsNumber").value) diff --git a/scripts/abins/io.py b/scripts/abins/io.py index 280ea1c6dd4a..c137cb6e22c5 100644 --- a/scripts/abins/io.py +++ b/scripts/abins/io.py @@ -35,7 +35,7 @@ class IO(BaseModel): setting: str = "" autoconvolution: int = 10 temperature: float = None - cache_directory: Path = None + cache_directory: Path | None = None @staticmethod def _dir_is_not_writeable(directory: str): diff --git a/scripts/abins/powdercalculator.py b/scripts/abins/powdercalculator.py index b34f61a7e8da..8dc7eff9076b 100644 --- a/scripts/abins/powdercalculator.py +++ b/scripts/abins/powdercalculator.py @@ -4,9 +4,11 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + -import numpy as np +from pathlib import Path from typing import Dict, Tuple +import numpy as np + import abins from abins.constants import ACOUSTIC_PHONON_THRESHOLD, CONSTANT, CM1_2_HARTREE, K_2_HARTREE, NUM_ZERO @@ -17,10 +19,11 @@ class PowderCalculator: Class for calculating powder data. """ - def __init__(self, *, filename: str, abins_data: abins.AbinsData, temperature: float) -> None: + def __init__(self, *, filename: str, abins_data: abins.AbinsData, temperature: float, cache_directory: Path | None = None) -> None: """ :param filename: name of input DFT filename :param abins_data: object of type AbinsData with data from input DFT file + :param temperature: temperature in Kelvin (for Bose-Einstein occupation) """ if not isinstance(abins_data, abins.AbinsData): raise ValueError("Object of AbinsData was expected.") @@ -40,7 +43,10 @@ def __init__(self, *, filename: str, abins_data: abins.AbinsData, temperature: f self._masses = np.asarray([atoms_data[atom]["mass"] for atom in range(len(atoms_data))]) self._clerk = abins.IO( - input_filename=filename, group_name=abins.parameters.hdf_groups["powder_data"], temperature=self._temperature + input_filename=filename, + group_name=abins.parameters.hdf_groups["powder_data"], + temperature=self._temperature, + cache_directory=cache_directory, ) def _calculate_powder(self) -> abins.PowderData: diff --git a/scripts/abins/scalculatorfactory.py b/scripts/abins/scalculatorfactory.py index 0c2292a72832..7f6c4ae960cb 100644 --- a/scripts/abins/scalculatorfactory.py +++ b/scripts/abins/scalculatorfactory.py @@ -4,6 +4,8 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + +from pathlib import Path + import numpy as np import abins @@ -28,6 +30,7 @@ def init( instrument: Instrument, quantum_order_num: int, autoconvolution_max: int = 0, + cache_directory: Path | None = None, ): """ :param filename: name of input DFT file (CASTEP: foo.phonon) @@ -37,6 +40,7 @@ def init( :param instrument: object of type Instrument for which simulation should be performed :param quantum_order_num: number of quantum order events taken into account during the simulation :param autoconvolution_max: Convolve results with fundamentals to obtain approximate spectra up to this order + :param cache_directory: Directory for .hdf5 cache files """ if sample_form in ALL_SAMPLE_FORMS: if sample_form == "Powder": @@ -47,6 +51,7 @@ def init( instrument=instrument, quantum_order_num=quantum_order_num, autoconvolution_max=autoconvolution_max, + cache_directory=cache_directory, ) else: diff --git a/scripts/abins/spowdersemiempiricalcalculator.py b/scripts/abins/spowdersemiempiricalcalculator.py index 17fbd7f0f0d2..bea91a2fa43e 100644 --- a/scripts/abins/spowdersemiempiricalcalculator.py +++ b/scripts/abins/spowdersemiempiricalcalculator.py @@ -5,6 +5,7 @@ # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + +from pathlib import Path from typing import Optional, Union import numpy as np @@ -34,6 +35,7 @@ def __init__( instrument: Instrument, quantum_order_num: int = Field(ge=1, le=2), autoconvolution_max: int = 0, + cache_directory: Path | None = None, ) -> None: """ :param filename: name of input DFT file (CASTEP: foo.phonon). This is only used for caching, the file will not be read. @@ -43,6 +45,7 @@ def __init__( :param quantum_order_num: number of quantum order events to simulate in semi-analytic approximation :param autoconvolution_max: approximate spectra up to this order using auto-convolution + :param cache_directory: location for .hdf5 """ from abins.constants import TWO_DIMENSIONAL_INSTRUMENTS @@ -53,6 +56,7 @@ def __init__( self._quantum_order_num = quantum_order_num self._autoconvolution_max = autoconvolution_max self._instrument = instrument + self._cache_directory = cache_directory # This is only used as metadata for clerk, like filename self._sample_form = "Powder" @@ -79,6 +83,7 @@ def __init__( sample_form=self._sample_form, temperature=self._temperature, ), + cache_directory=self._cache_directory, ) # Set up two sampling grids: _bins for broadening/output @@ -245,7 +250,7 @@ def _calculate_s(self) -> SData: # Compute tensors and traces, write to cache for access during atomic s calculations powder_calculator = abins.PowderCalculator( - filename=self._input_filename, abins_data=self._abins_data, temperature=self._temperature + filename=self._input_filename, abins_data=self._abins_data, temperature=self._temperature, cache_directory=self._cache_directory ) self._powder_data = powder_calculator.get_formatted_data() From f233f45c23dbce232f661c722be24d27c8acf948 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Wed, 15 Jan 2025 16:51:44 +0000 Subject: [PATCH 07/19] Ensure explicit cache_directory is used across Abins and tests - Temporarily raise an error if the default cache_directory (None) is passed to abins.io.IO - This helped identify where tests were using default dirs. Now there are updated to use a (somewhat inconsistent) mixture of temporary directories and the user default directory --- scripts/abins/abinsdata.py | 2 +- scripts/abins/input/abinitioloader.py | 8 +++- scripts/abins/input/crystalloader.py | 6 ++- scripts/abins/input/dmol3loader.py | 7 --- scripts/abins/input/gaussianloader.py | 5 +- scripts/abins/input/tester.py | 46 +++++++++++-------- scripts/abins/io.py | 3 ++ .../test/Abins/AbinsCalculateSPowderTest.py | 22 +++++++-- scripts/test/Abins/AbinsDataTest.py | 3 +- scripts/test/Abins/AbinsIOmoduleTest.py | 21 +++++---- scripts/test/Abins/AbinsLoadCASTEPTest.py | 14 ++++-- scripts/test/Abins/AbinsLoadVASPTest.py | 31 ++++++++----- .../test/Abins/AbinsPowderCalculatorTest.py | 30 +++++++----- 13 files changed, 124 insertions(+), 74 deletions(-) diff --git a/scripts/abins/abinsdata.py b/scripts/abins/abinsdata.py index 94f22496d177..89c3136a9f27 100644 --- a/scripts/abins/abinsdata.py +++ b/scripts/abins/abinsdata.py @@ -49,7 +49,7 @@ def from_calculation_data(filename: str, ab_initio_program: str, cache_directory ab_initio_program.upper(), " ".join(all_loaders.keys()) ) ) - loader = all_loaders[ab_initio_program.upper()](input_ab_initio_filename=filename) + loader = all_loaders[ab_initio_program.upper()](input_ab_initio_filename=filename, cache_directory=cache_directory) data = loader.get_formatted_data() return data diff --git a/scripts/abins/input/abinitioloader.py b/scripts/abins/input/abinitioloader.py index f578ab79339f..1c7a4d78523c 100644 --- a/scripts/abins/input/abinitioloader.py +++ b/scripts/abins/input/abinitioloader.py @@ -30,7 +30,7 @@ class AbInitioLoader(metaclass=NamedAbstractClass): read_formatted_data() if necessary and caching the results. """ - def __init__(self, input_ab_initio_filename: str = None): + def __init__(self, input_ab_initio_filename: str = None, cache_directory: Path | None = None): """An object for loading phonon data from ab initio output files""" if not isinstance(input_ab_initio_filename, str): @@ -38,7 +38,11 @@ def __init__(self, input_ab_initio_filename: str = None): elif not Path(input_ab_initio_filename).is_file(): raise IOError(f"Ab initio file {input_ab_initio_filename} not found.") - self._clerk = abins.IO(input_filename=input_ab_initio_filename, group_name=abins.parameters.hdf_groups["ab_initio_data"]) + self._clerk = abins.IO( + input_filename=input_ab_initio_filename, + group_name=abins.parameters.hdf_groups["ab_initio_data"], + cache_directory=cache_directory, + ) @property @abstractmethod diff --git a/scripts/abins/input/crystalloader.py b/scripts/abins/input/crystalloader.py index c70c8336f625..27a4a7712eff 100644 --- a/scripts/abins/input/crystalloader.py +++ b/scripts/abins/input/crystalloader.py @@ -5,6 +5,8 @@ # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + import io +from pathlib import Path + import numpy as np from .textparser import TextParser @@ -19,11 +21,11 @@ class CRYSTALLoader(AbInitioLoader): contributing to this module. """ - def __init__(self, input_ab_initio_filename=None): + def __init__(self, input_ab_initio_filename=None, cache_directory: Path | None = None): """ :param input_ab_initio_filename: name of a file with vibrational or phonon data (foo.out) """ - super().__init__(input_ab_initio_filename=input_ab_initio_filename) + super().__init__(input_ab_initio_filename=input_ab_initio_filename, cache_directory=cache_directory) self._num_k = None self._num_modes = None diff --git a/scripts/abins/input/dmol3loader.py b/scripts/abins/input/dmol3loader.py index 173a63b3c9c5..afc842d80eaa 100644 --- a/scripts/abins/input/dmol3loader.py +++ b/scripts/abins/input/dmol3loader.py @@ -37,13 +37,6 @@ class DMOL3Loader(AbInitioLoader): Class for loading DMOL3 ab initio vibrational data. """ - def __init__(self, input_ab_initio_filename): - """ - :param input_ab_initio_filename: name of file with vibrational data (foo.outmol) - """ - super().__init__(input_ab_initio_filename=input_ab_initio_filename) - self._norm = 0 - @property def _ab_initio_program(self) -> str: return "DMOL3" diff --git a/scripts/abins/input/gaussianloader.py b/scripts/abins/input/gaussianloader.py index fa44df68e1f7..632e04d86b5e 100644 --- a/scripts/abins/input/gaussianloader.py +++ b/scripts/abins/input/gaussianloader.py @@ -6,6 +6,7 @@ # SPDX - License - Identifier: GPL - 3.0 + import io from io import BufferedReader +from pathlib import Path import re from typing import Iterable, List @@ -22,11 +23,11 @@ class GAUSSIANLoader(AbInitioLoader): Class for loading GAUSSIAN ab initio vibrational data. """ - def __init__(self, input_ab_initio_filename) -> None: + def __init__(self, input_ab_initio_filename: str, cache_directory: Path | None = None) -> None: """ :param input_ab_initio_filename: name of file with vibrational data (foo.log or foo.LOG) """ - super().__init__(input_ab_initio_filename=input_ab_initio_filename) + super().__init__(input_ab_initio_filename=input_ab_initio_filename, cache_directory=cache_directory) self._active_atoms = None self._num_atoms = None self._num_read_freq = 0 diff --git a/scripts/abins/input/tester.py b/scripts/abins/input/tester.py index e034f30e5900..2ab88842f9cb 100644 --- a/scripts/abins/input/tester.py +++ b/scripts/abins/input/tester.py @@ -6,6 +6,8 @@ # SPDX - License - Identifier: GPL - 3.0 + import json from numbers import Real +from pathlib import Path +from tempfile import TemporaryDirectory from typing import Any, Dict import numpy as np @@ -134,10 +136,16 @@ def _check_reader_data(self, correct_data=None, data=None, filename=None, extens assert_allclose(correct_data["datasets"]["unit_cell"], data["datasets"]["unit_cell"]) def _check_loader_data( - self, correct_data=None, input_ab_initio_filename=None, extension=None, loader=None, max_displacement_kpt: Real = float("Inf") + self, + correct_data=None, + input_ab_initio_filename=None, + extension=None, + loader=None, + max_displacement_kpt: Real = float("Inf"), + cache_directory: Path | None = None, ): read_filename = abins.test_helpers.find_file(f"{input_ab_initio_filename}.{extension}") - ab_initio_loader = loader(input_ab_initio_filename=read_filename) + ab_initio_loader = loader(input_ab_initio_filename=read_filename, cache_directory=cache_directory) abins_data = ab_initio_loader.load_formatted_data() self.assertTrue(abins_data.get_kpoints_data().is_normalised()) @@ -189,25 +197,27 @@ def check( if extension is None: extension = self._loaders_extensions[str(loader)] - # get calculated data - data = self._read_ab_initio(loader=loader, filename=name, extension=extension, **loader_kwargs) - # get correct data correct_data = self._prepare_data(name, max_displacement_kpt=max_displacement_kpt) - # check read data - self._check_reader_data( - correct_data=correct_data, data=data, filename=name, extension=extension, max_displacement_kpt=max_displacement_kpt - ) - - # check loaded data - self._check_loader_data( - correct_data=correct_data, - input_ab_initio_filename=name, - extension=extension, - loader=loader, - max_displacement_kpt=max_displacement_kpt, - ) + with TemporaryDirectory() as tmpdir: + # get calculated data + data = self._read_ab_initio(loader=loader, filename=name, extension=extension, cache_directory=Path(tmpdir), **loader_kwargs) + + # check read data + self._check_reader_data( + correct_data=correct_data, data=data, filename=name, extension=extension, max_displacement_kpt=max_displacement_kpt + ) + + # check loaded data + self._check_loader_data( + correct_data=correct_data, + input_ab_initio_filename=name, + extension=extension, + loader=loader, + max_displacement_kpt=max_displacement_kpt, + cache_directory=Path(tmpdir), + ) def _read_ab_initio(self, loader=None, filename=None, extension=None, **loader_kwargs) -> Dict[str, Any]: """ diff --git a/scripts/abins/io.py b/scripts/abins/io.py index c137cb6e22c5..7c58a95d2874 100644 --- a/scripts/abins/io.py +++ b/scripts/abins/io.py @@ -60,6 +60,9 @@ def get_save_dir_path(self): if self.cache_directory: return self.cache_directory + else: + raise Exception("No directory") + return Path(ConfigService.getString("defaultsave.directory")) def model_post_init(self, __context): diff --git a/scripts/test/Abins/AbinsCalculateSPowderTest.py b/scripts/test/Abins/AbinsCalculateSPowderTest.py index 4ca0a2821918..132d4fe57acc 100644 --- a/scripts/test/Abins/AbinsCalculateSPowderTest.py +++ b/scripts/test/Abins/AbinsCalculateSPowderTest.py @@ -7,7 +7,9 @@ from copy import deepcopy import functools import json +from pathlib import Path import unittest +from tempfile import TemporaryDirectory import numpy as np from numpy.testing import assert_almost_equal @@ -32,14 +34,21 @@ class SCalculatorFactoryPowderTest(unittest.TestCase): _si2 = "Si2-sc_CalculateSPowder" _instruments_defaults = {} - default_calculator_kwargs = dict( - temperature=_temperature, instrument=_instrument, sample_form=_sample_form, quantum_order_num=_order_event, autoconvolution_max=0 - ) - def setUp(self): self.default_threads = abins.parameters.performance["threads"] abins.parameters.performance["threads"] = 1 self._instruments_defaults = deepcopy(abins.parameters.instruments) + self._temporary_directory = TemporaryDirectory() + self._cache_directory = Path(self._temporary_directory.name) + + self.default_calculator_kwargs = dict( + temperature=self._temperature, + instrument=self._instrument, + sample_form=self._sample_form, + quantum_order_num=self._order_event, + autoconvolution_max=0, + cache_directory=self._cache_directory, + ) def tearDown(self): from mantid.kernel import ConfigService @@ -65,6 +74,7 @@ def test_invalid_input(self): abins_data=good_data, instrument=self._instrument, quantum_order_num=self._order_event, + cache_directory=self._cache_directory, ) # invalid temperature @@ -76,6 +86,7 @@ def test_invalid_input(self): abins_data=good_data, instrument=self._instrument, quantum_order_num=self._order_event, + cache_directory=self._cache_directory, ) # invalid sample @@ -87,6 +98,7 @@ def test_invalid_input(self): abins_data=good_data, instrument=self._instrument, quantum_order_num=self._order_event, + cache_directory=self._cache_directory, ) # invalid abins data: content of abins data instead of object abins_data @@ -98,6 +110,7 @@ def test_invalid_input(self): abins_data=good_data.extract(), instrument=self._instrument, quantum_order_num=self._order_event, + cache_directory=self._cache_directory, ) # invalid instrument @@ -109,6 +122,7 @@ def test_invalid_input(self): abins_data=good_data, instrument="INSTRUMENT", quantum_order_num=self._order_event, + cache_directory=self._cache_directory, ) def test_1d_order1(self): diff --git a/scripts/test/Abins/AbinsDataTest.py b/scripts/test/Abins/AbinsDataTest.py index cc402dc3140d..c4abe930dd5b 100644 --- a/scripts/test/Abins/AbinsDataTest.py +++ b/scripts/test/Abins/AbinsDataTest.py @@ -51,8 +51,9 @@ def test_data_content(self): class DummyLoader: - def __init__(self, *, input_ab_initio_filename): + def __init__(self, *, input_ab_initio_filename, cache_directory): self.filename = input_ab_initio_filename + self.cache_directory = cache_directory def get_formatted_data(self): return "FORMATTED DATA" diff --git a/scripts/test/Abins/AbinsIOmoduleTest.py b/scripts/test/Abins/AbinsIOmoduleTest.py index 29cc0cb0de3f..9bee9e0fecc8 100644 --- a/scripts/test/Abins/AbinsIOmoduleTest.py +++ b/scripts/test/Abins/AbinsIOmoduleTest.py @@ -4,6 +4,7 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + +from pathlib import Path import unittest import numpy as np @@ -12,14 +13,16 @@ class IOTest(unittest.TestCase): - def tearDown(self): + def setUp(self): from mantid.kernel import ConfigService - test_helpers.remove_output_files(list_of_names=["Cars", "temphgfrt"], directory=ConfigService.getString("defaultsave.directory")) + self._cache_directory = Path(ConfigService.getString("defaultsave.directory")) + + def tearDown(self): + test_helpers.remove_output_files(list_of_names=["Cars", "temphgfrt"], directory=self._cache_directory) - @staticmethod - def _save_stuff(): - saver = IO(input_filename="Cars.foo", group_name="Volksvagen") + def _save_stuff(self): + saver = IO(input_filename="Cars.foo", group_name="Volksvagen", cache_directory=self._cache_directory) # add some attributes saver.add_attribute("Fuel", 100) @@ -45,13 +48,13 @@ def _save_stuff(): saver.save() def _add_wrong_attribute(self): - poor_saver = IO(input_filename="BadCars.foo", group_name="Volksvagen") + poor_saver = IO(input_filename="BadCars.foo", group_name="Volksvagen", cache_directory=self._cache_directory) with self.assertRaisesRegex(ValidationError, "Input should be an instance of int64"): poor_saver.add_attribute("BadPassengers", np.array([4, 5], dtype=np.int64)) def _save_wrong_dataset(self): - poor_saver = IO(input_filename="BadCars.foo", group_name="Volksvagen") + poor_saver = IO(input_filename="BadCars.foo", group_name="Volksvagen", cache_directory=self._cache_directory) poor_saver.add_data("BadPassengers", 4) self.assertRaises(TypeError, poor_saver.save) @@ -65,7 +68,7 @@ def _wrong_groupname(self): self.assertRaises(ValueError, IO, input_filename="goodfile", group_name=1) def _wrong_file(self): - poor_loader = IO(input_filename="bumCars", group_name="nice_group") + poor_loader = IO(input_filename="bumCars", group_name="nice_group", cache_directory=self._cache_directory) self.assertRaises(IOError, poor_loader.load, list_of_attributes="one_attribute") def _loading_attributes(self): @@ -116,7 +119,7 @@ def runTest(self): self._add_wrong_attribute() self._save_wrong_dataset() - self.loader = IO(input_filename="Cars.foo", group_name="Volksvagen") + self.loader = IO(input_filename="Cars.foo", group_name="Volksvagen", cache_directory=self._cache_directory) self._wrong_filename_type() self._empty_filename() diff --git a/scripts/test/Abins/AbinsLoadCASTEPTest.py b/scripts/test/Abins/AbinsLoadCASTEPTest.py index 5424778ba886..08943d35db0c 100644 --- a/scripts/test/Abins/AbinsLoadCASTEPTest.py +++ b/scripts/test/Abins/AbinsLoadCASTEPTest.py @@ -4,6 +4,8 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + +from pathlib import Path +from tempfile import TemporaryDirectory import unittest from unittest.mock import patch, Mock @@ -23,12 +25,14 @@ def test_euphonic_castep_call(self, from_castep): # filename makes it to Euphonic CASTEP parser filename = abins.test_helpers.find_file("squaricn_sum_LoadCASTEP.phonon") - reader = CASTEPLoader(input_ab_initio_filename=filename) - try: - reader.read_vibrational_or_phonon_data() - except TypeError: - pass # Clerk will freak out when passed mocked data to serialise + with TemporaryDirectory() as tmpdir: + reader = CASTEPLoader(input_ab_initio_filename=filename, cache_directory=Path(tmpdir)) + + try: + reader.read_vibrational_or_phonon_data() + except TypeError: + pass # Clerk will freak out when passed mocked data to serialise from_castep.assert_called_with(filename, prefer_non_loto=True) diff --git a/scripts/test/Abins/AbinsLoadVASPTest.py b/scripts/test/Abins/AbinsLoadVASPTest.py index 932d01fa55e7..a677a4e00fad 100644 --- a/scripts/test/Abins/AbinsLoadVASPTest.py +++ b/scripts/test/Abins/AbinsLoadVASPTest.py @@ -4,33 +4,40 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + +from pathlib import Path import unittest +from tempfile import TemporaryDirectory import abins from abins.input import VASPLoader class AbinsLoadVASPTest(unittest.TestCase, abins.input.Tester): - def tearDown(self): - # Remove cache files - from mantid.kernel import ConfigService + # def tearDown(self): + # # Remove cache files + # from mantid.kernel import ConfigService - abins.test_helpers.remove_output_files(list_of_names=["_LoadVASP"], directory=ConfigService.getString("defaultsave.directory")) + # abins.test_helpers.remove_output_files(list_of_names=["_LoadVASP"], directory=ConfigService.getString("defaultsave.directory")) def test_non_existing_file(self): - with self.assertRaises(IOError): - bad_vasp_reader = VASPLoader(input_ab_initio_filename="NonExistingFile.txt") - bad_vasp_reader.read_vibrational_or_phonon_data() + with TemporaryDirectory() as tmpdir: + with self.assertRaises(IOError): + bad_vasp_reader = VASPLoader(input_ab_initio_filename="NonExistingFile.txt", cache_directory=Path(tmpdir)) + bad_vasp_reader.read_vibrational_or_phonon_data() - with self.assertRaises(TypeError): - _ = VASPLoader(input_ab_initio_filename=1) + with self.assertRaises(TypeError): + _ = VASPLoader(input_ab_initio_filename=1, cache_directory=Path(tmpdir)) # Not a real vibration calc; check the appropriate error is raised def test_singlepoint_input(self): filename = abins.test_helpers.find_file("ethane_singlepoint.xml") - bad_vasp_reader = VASPLoader(input_ab_initio_filename=filename) - with self.assertRaisesRegexp(ValueError, "Could not find a 'calculation' block containing a 'dynmat' block in VASP XML file\\."): - bad_vasp_reader.read_vibrational_or_phonon_data() + + with TemporaryDirectory() as tmpdir: + bad_vasp_reader = VASPLoader(input_ab_initio_filename=filename, cache_directory=Path(tmpdir)) + with self.assertRaisesRegexp( + ValueError, "Could not find a 'calculation' block containing a 'dynmat' block in VASP XML file\\." + ): + bad_vasp_reader.read_vibrational_or_phonon_data() # IBRION=8 from optimised structure def test_xml_dfpt(self): diff --git a/scripts/test/Abins/AbinsPowderCalculatorTest.py b/scripts/test/Abins/AbinsPowderCalculatorTest.py index ea340fe1fa33..51503c5c7b0e 100644 --- a/scripts/test/Abins/AbinsPowderCalculatorTest.py +++ b/scripts/test/Abins/AbinsPowderCalculatorTest.py @@ -4,10 +4,12 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + +import json +from pathlib import Path import unittest + import numpy as np from numpy.testing import assert_allclose -import json from abins import AbinsData, PowderCalculator import abins.input @@ -22,12 +24,13 @@ class PowderCalculatorTest(unittest.TestCase): _si2 = "Si2-sc_CalculatePowder" # test input - def tearDown(self): + def setUp(self): from mantid.kernel import ConfigService - abins.test_helpers.remove_output_files( - list_of_names=["CalculatePowder"], directory=ConfigService.getString("defaultsave.directory") - ) + self._cache_directory = Path(ConfigService.getString("defaultsave.directory")) + + def tearDown(self): + abins.test_helpers.remove_output_files(list_of_names=["CalculatePowder"], directory=self._cache_directory) def test_wrong_input(self): full_path_filename = abins.test_helpers.find_file(filename=self._si2 + ".json") @@ -55,7 +58,9 @@ def _good_case(self, name=None): abins_data_filename = abins.test_helpers.find_file(filename=name + ".json") ref_data = self._get_ref_data(filename=abins_data_filename) - good_tester = abins.PowderCalculator(filename=abins_data_filename, abins_data=ref_data["DFT"], temperature=300.0) + good_tester = abins.PowderCalculator( + filename=abins_data_filename, abins_data=ref_data["DFT"], temperature=300.0, cache_directory=self._cache_directory + ) calculated_data = good_tester.calculate_data().extract() # check if evaluated powder data is correct @@ -67,7 +72,9 @@ def _good_case(self, name=None): raise AssertionError(f"Difference in field '{key}' for case {name}") from e # check if loading powder data is correct - new_tester = abins.PowderCalculator(filename=abins_data_filename, abins_data=ref_data["DFT"], temperature=300.0) + new_tester = abins.PowderCalculator( + filename=abins_data_filename, abins_data=ref_data["DFT"], temperature=300.0, cache_directory=self._cache_directory + ) loaded_data = new_tester.load_formatted_data().extract() @@ -88,17 +95,18 @@ def test_write_reference_data(self): self._write_data(name=self._c6h6) self._write_data(name=self._si2) - @classmethod - def _write_data(cls, name=None): + def _write_data(self, name=None): abins_data_filename = abins.test_helpers.find_file(name + ".json") with open(abins_data_filename, "r") as fp: abins_data = AbinsData.from_dict(json.load(fp)) - powder = abins.PowderCalculator(filename=abins_data_filename, abins_data=abins_data, temperature=300.0).calculate_data() + powder = abins.PowderCalculator( + filename=abins_data_filename, abins_data=abins_data, temperature=300.0, cache_directory=self._cache_directory + ).calculate_data() powder_dict = abins.test_helpers.dict_arrays_to_lists(powder.extract()) json_file = abins_data_filename.replace(".json", "_powder.txt") - raise Exception(json_file) + with open(json_file, "w") as fd: json.dump(powder_dict, fd, indent=4, sort_keys=True) From 24d9be9ffdd595da6e57c5263804ef07684b3fcc Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Fri, 17 Jan 2025 11:07:26 +0000 Subject: [PATCH 08/19] More use of tmpdir on Abins tests - With explicit cache paths we can remove the need to patch IO class and simplify tests - Using TemporaryDirectory.cleanup() simplifies post-test clean-up --- .../plugins/algorithms/Abins2BasicTest.py | 264 +++++++++--------- .../algorithms/AbinsAdvancedParametersTest.py | 16 +- .../plugins/algorithms/AbinsBasicTest.py | 24 +- .../test/Abins/AbinsCalculateSPowderTest.py | 5 +- .../test/Abins/AbinsPowderCalculatorTest.py | 8 +- 5 files changed, 150 insertions(+), 167 deletions(-) diff --git a/Framework/PythonInterface/test/python/plugins/algorithms/Abins2BasicTest.py b/Framework/PythonInterface/test/python/plugins/algorithms/Abins2BasicTest.py index c2eb2898204d..660c94b81c34 100644 --- a/Framework/PythonInterface/test/python/plugins/algorithms/Abins2BasicTest.py +++ b/Framework/PythonInterface/test/python/plugins/algorithms/Abins2BasicTest.py @@ -5,14 +5,10 @@ # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + import logging -from pathlib import Path import tempfile import unittest -from unittest.mock import patch from numpy.testing import assert_array_almost_equal -import abins -import abins.io from abins.abins2 import Abins as Abins2 from abins.constants import ATOM_PREFIX @@ -23,6 +19,10 @@ class AbinsBasicTest(unittest.TestCase): + # Test-related parameters + _tolerance = 0.0001 + _tmp_cache_dir = None + # Define typical values for parameters _si2 = "Si2-sc_Abins" _squaricn = "squaricn_sum_Abins" @@ -36,16 +36,10 @@ class AbinsBasicTest(unittest.TestCase): _cross_section_factor = "Incoherent" _workspace_name = "output_workspace" - # Test-related parameters - _tolerance = 0.0001 - _tmp_cache_dir = None - - def get_cache_dir(self): - return Path(self._tmp_cache_dir.name) - @classmethod def setUpClass(cls): cls._tmp_cache_dir = tempfile.TemporaryDirectory() + cls._cache_directory = cls._tmp_cache_dir.name @classmethod def tearDownClass(cls): @@ -55,18 +49,6 @@ def setUp(self): self.logger = logging.getLogger("abins2-basic-test") def tearDown(self): - abins.test_helpers.remove_output_files( - list_of_names=[ - "explicit", - "default", - "total", - "squaricn_sum_Abins", - "benzene_exp", - "benzene_Abins", - "numbered", - ], - directory=self.get_cache_dir(), - ) mtd.clear() def test_subscribe_warning(self): @@ -80,12 +62,15 @@ def test_wrong_input(self): """Test if the correct behaviour of algorithm in case input is not valid""" # invalid CASTEP file missing: Number of branches 6 in the header file - with ( self.assertRaisesRegex(RuntimeError, "The third line should include 'Number of branches'."), - patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir), ): - Abins(Version=2, VibrationalOrPhononFile="Si2-sc_wrong.phonon", OutputWorkspace=self._workspace_name) + Abins( + Version=2, + VibrationalOrPhononFile="Si2-sc_wrong.phonon", + OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, + ) # wrong extension of phonon file in case of CASTEP with ( @@ -93,12 +78,12 @@ def test_wrong_input(self): RuntimeError, "The expected extension of file is .phonon.", ), - patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir), ): Abins( Version=2, VibrationalOrPhononFile="Si2-sc.wrong_phonon", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) # wrong extension of phonon file in case of CRYSTAL @@ -107,18 +92,23 @@ def test_wrong_input(self): RuntimeError, "The expected extension of file is .out.", ), - patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir), ): Abins( Version=2, AbInitioProgram="CRYSTAL", VibrationalOrPhononFile="MgO.wrong_out", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) # no name for workspace - with self.assertRaises(TypeError), patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir): - Abins(Version=2, VibrationalOrPhononFile=self._si2 + ".phonon", TemperatureInKelvin=self._temperature) + with self.assertRaises(TypeError): + Abins( + Version=2, + VibrationalOrPhononFile=self._si2 + ".phonon", + TemperatureInKelvin=self._temperature, + CacheDirectory=self._cache_directory, + ) # keyword total in the name of the workspace with ( @@ -126,25 +116,25 @@ def test_wrong_input(self): RuntimeError, "Keyword: total cannot be used in the name of workspace.", ), - patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir), ): Abins( Version=2, VibrationalOrPhononFile=self._si2 + ".phonon", TemperatureInKelvin=self._temperature, OutputWorkspace=self._workspace_name + "total", + CacheDirectory=self._cache_directory, ) # non-existent parameter with ( self.assertRaisesRegex(TypeError, "'SampleForm' is an invalid keyword argument"), - patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir), ): Abins( Version=2, VibrationalOrPhononFile=self._si2 + ".phonon", OutputWorkspace=self._workspace_name, SampleForm="Powder", + CacheDirectory=self._cache_directory, ) # negative temperature in K @@ -153,25 +143,25 @@ def test_wrong_input(self): RuntimeError, "Temperature must be positive.", ), - patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir), ): Abins( Version=2, VibrationalOrPhononFile=self._si2 + ".phonon", TemperatureInKelvin=-1.0, OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) # unknown instrument with ( self.assertRaisesRegex(ValueError, 'The value "UnknownInstrument" is not in the list of allowed values'), - patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir), ): Abins( Version=2, VibrationalOrPhononFile=self._si2 + ".phonon", Instrument="UnknownInstrument", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) # test if intermediate results are consistent @@ -181,13 +171,13 @@ def test_non_unique_elements(self): """ with ( self.assertRaisesRegex(RuntimeError, r"User atom selection \(by symbol\) contains repeated species."), - patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir), ): Abins( Version=2, VibrationalOrPhononFile=self._squaricn + ".phonon", Atoms="C,C,H", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) def test_non_unique_atoms(self): @@ -196,13 +186,13 @@ def test_non_unique_atoms(self): """ with ( self.assertRaisesRegex(RuntimeError, r"User atom selection \(by number\) contains repeated atom."), - patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir), ): Abins( Version=2, VibrationalOrPhononFile=self._squaricn + ".phonon", Atoms="atom_1,atom_2,atom1", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) def test_non_existing_atoms(self): @@ -212,108 +202,111 @@ def test_non_existing_atoms(self): # In _squaricn there are no N atoms with ( self.assertRaisesRegex(RuntimeError, r"User defined atom selection \(by element\) 'N': not present in the system."), - patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir), ): Abins( Version=2, VibrationalOrPhononFile=self._squaricn + ".phonon", Atoms="N", OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, ) def test_atom_index_limits(self): """Individual atoms may be indexed (counting from 1); if the index falls outside number of atoms, Abins should terminate with a useful error message. """ - with patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir): - self.assertRaisesRegex( - RuntimeError, - r"Invalid user atom selection \(by number\)" + f" '{ATOM_PREFIX}0'", - Abins, - VibrationalOrPhononFile=self._squaricn + ".phonon", - Atoms=ATOM_PREFIX + "0", - OutputWorkspace=self._workspace_name, - ) - self.assertRaisesRegex( - RuntimeError, - r"Invalid user atom selection \(by number\)" + f" '{ATOM_PREFIX}61'", - Abins, - VibrationalOrPhononFile=self._squaricn + ".phonon", - Atoms=ATOM_PREFIX + "61", - OutputWorkspace=self._workspace_name, - ) + self.assertRaisesRegex( + RuntimeError, + r"Invalid user atom selection \(by number\)" + f" '{ATOM_PREFIX}0'", + Abins, + VibrationalOrPhononFile=self._squaricn + ".phonon", + Atoms=ATOM_PREFIX + "0", + OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, + ) + self.assertRaisesRegex( + RuntimeError, + r"Invalid user atom selection \(by number\)" + f" '{ATOM_PREFIX}61'", + Abins, + VibrationalOrPhononFile=self._squaricn + ".phonon", + Atoms=ATOM_PREFIX + "61", + OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, + ) def test_atom_index_invalid(self): r"""If the atoms field includes an unmatched entry (i.e. containing the prefix but not matching the '\d+' regex, Abins should terminate with a useful error message. """ - with patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir): - self.assertRaisesRegex( - RuntimeError, - r"Not all user atom selections \('atoms' option\) were understood.", - Abins, - VibrationalOrPhononFile=self._squaricn + ".phonon", - Atoms=ATOM_PREFIX + "-3", - OutputWorkspace=self._workspace_name, - ) - self.assertRaisesRegex( - RuntimeError, - r"Not all user atom selections \('atoms' option\) were understood.", - Abins, - VibrationalOrPhononFile=self._squaricn + ".phonon", - Atoms=ATOM_PREFIX + "_#4", - OutputWorkspace=self._workspace_name, - ) + self.assertRaisesRegex( + RuntimeError, + r"Not all user atom selections \('atoms' option\) were understood.", + Abins, + VibrationalOrPhononFile=self._squaricn + ".phonon", + Atoms=ATOM_PREFIX + "-3", + OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, + ) + self.assertRaisesRegex( + RuntimeError, + r"Not all user atom selections \('atoms' option\) were understood.", + Abins, + VibrationalOrPhononFile=self._squaricn + ".phonon", + Atoms=ATOM_PREFIX + "_#4", + OutputWorkspace=self._workspace_name, + CacheDirectory=self._cache_directory, + ) def test_lagrange_exists(self): - with patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir): - Abins( - Version=2, - AbInitioProgram=self._ab_initio_program, - VibrationalOrPhononFile=(self._squaricn + ".phonon"), - TemperatureInKelvin=self._temperature, - Instrument="Lagrange", - Setting="Cu(331) (Lagrange)", - Atoms=self._atoms, - SumContributions=self._sum_contributions, - QuantumOrderEventsNumber=self._quantum_order_events_number, - ScaleByCrossSection=self._cross_section_factor, - OutputWorkspace="squaricn-lagrange", - ) + Abins( + Version=2, + AbInitioProgram=self._ab_initio_program, + VibrationalOrPhononFile=(self._squaricn + ".phonon"), + TemperatureInKelvin=self._temperature, + Instrument="Lagrange", + Setting="Cu(331) (Lagrange)", + Atoms=self._atoms, + SumContributions=self._sum_contributions, + QuantumOrderEventsNumber=self._quantum_order_events_number, + ScaleByCrossSection=self._cross_section_factor, + OutputWorkspace="squaricn-lagrange", + CacheDirectory=self._cache_directory, + ) def test_partial_by_element(self): """Check results of INS spectrum resolved by elements: default should match explicit list of elements""" + wrk_ref = Abins( + Version=2, + AbInitioProgram=self._ab_initio_program, + VibrationalOrPhononFile=self._squaricn + ".phonon", + TemperatureInKelvin=self._temperature, + Instrument=self._instrument_name, + Atoms=self._atoms, + SumContributions=self._sum_contributions, + QuantumOrderEventsNumber=self._quantum_order_events_number, + ScaleByCrossSection=self._cross_section_factor, + OutputWorkspace=self._squaricn + "_ref", + CacheDirectory=self._cache_directory, + ) - with patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir): - wrk_ref = Abins( - Version=2, - AbInitioProgram=self._ab_initio_program, - VibrationalOrPhononFile=self._squaricn + ".phonon", - TemperatureInKelvin=self._temperature, - Instrument=self._instrument_name, - Atoms=self._atoms, - SumContributions=self._sum_contributions, - QuantumOrderEventsNumber=self._quantum_order_events_number, - ScaleByCrossSection=self._cross_section_factor, - OutputWorkspace=self._squaricn + "_ref", - ) - - wks_all_atoms_explicitly = Abins( - Version=2, - VibrationalOrPhononFile=self._squaricn + ".phonon", - Atoms="H, C, O", - SumContributions=self._sum_contributions, - QuantumOrderEventsNumber=self._quantum_order_events_number, - OutputWorkspace="explicit", - ) + wks_all_atoms_explicitly = Abins( + Version=2, + VibrationalOrPhononFile=self._squaricn + ".phonon", + Atoms="H, C, O", + SumContributions=self._sum_contributions, + QuantumOrderEventsNumber=self._quantum_order_events_number, + OutputWorkspace="explicit", + CacheDirectory=self._cache_directory, + ) - wks_all_atoms_default = Abins( - Version=2, - VibrationalOrPhononFile=self._squaricn + ".phonon", - SumContributions=self._sum_contributions, - QuantumOrderEventsNumber=self._quantum_order_events_number, - OutputWorkspace="default", - ) + wks_all_atoms_default = Abins( + Version=2, + VibrationalOrPhononFile=self._squaricn + ".phonon", + SumContributions=self._sum_contributions, + QuantumOrderEventsNumber=self._quantum_order_events_number, + OutputWorkspace="default", + CacheDirectory=self._cache_directory, + ) # Python 3 has no guarantee of dict order so the workspaces in the group may be in # a different order on Python 3 @@ -335,28 +328,29 @@ def test_partial_by_element(self): def test_partial_by_number(self): """Simulated INS spectrum can also be resolved by numbered atoms. Check consistency with element totals""" - with patch.object(abins.io.IO, "get_save_dir_path", side_effect=self.get_cache_dir): - wrk_ref = Abins( - Version=2, - AbInitioProgram=self._ab_initio_program, - VibrationalOrPhononFile=self._squaricn + ".phonon", - Atoms=self._atoms, - QuantumOrderEventsNumber=self._quantum_order_events_number, - ScaleByCrossSection=self._cross_section_factor, - OutputWorkspace=self._squaricn + "_ref", - ) + wrk_ref = Abins( + Version=2, + AbInitioProgram=self._ab_initio_program, + VibrationalOrPhononFile=self._squaricn + ".phonon", + Atoms=self._atoms, + QuantumOrderEventsNumber=self._quantum_order_events_number, + ScaleByCrossSection=self._cross_section_factor, + OutputWorkspace=self._squaricn + "_ref", + CacheDirectory=self._cache_directory, + ) - numbered_workspace_name = "numbered" - h_indices = ("1", "2", "3", "4") - wks_numbered_atoms = Abins( - Version=2, - VibrationalOrPhononFile=self._squaricn + ".phonon", - Atoms=", ".join([ATOM_PREFIX + s for s in h_indices]), - SumContributions=self._sum_contributions, - QuantumOrderEventsNumber=self._quantum_order_events_number, - ScaleByCrossSection=self._cross_section_factor, - OutputWorkspace=numbered_workspace_name, - ) + numbered_workspace_name = "numbered" + h_indices = ("1", "2", "3", "4") + wks_numbered_atoms = Abins( + Version=2, + VibrationalOrPhononFile=self._squaricn + ".phonon", + Atoms=", ".join([ATOM_PREFIX + s for s in h_indices]), + SumContributions=self._sum_contributions, + QuantumOrderEventsNumber=self._quantum_order_events_number, + ScaleByCrossSection=self._cross_section_factor, + OutputWorkspace=numbered_workspace_name, + CacheDirectory=self._cache_directory, + ) wrk_ref_names = list(wrk_ref.getNames()) wrk_h_total = wrk_ref[wrk_ref_names.index(self._squaricn + "_ref_H_total")] diff --git a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py index c06a6db14fa7..812cb07d94c8 100644 --- a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py +++ b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py @@ -4,12 +4,10 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + -from pathlib import Path +from tempfile import TemporaryDirectory import unittest from mantid.simpleapi import Abins, mtd -from mantid.kernel import ConfigService -from abins import test_helpers import abins.parameters try: @@ -25,7 +23,8 @@ def setUp(self): # set up input for Abins self._Si2 = "Si2-sc_AbinsAdvancedParameters" self._wrk_name = self._Si2 + "_ref" - self._cache_directory = ConfigService.getString("defaultsave.directory") + self._tmpdir = TemporaryDirectory() + self._cache_directory = self._tmpdir.name # before each test set abins.parameters to default values abins.parameters.instruments = { @@ -56,7 +55,7 @@ def setUp(self): abins.parameters.performance = {"optimal_size": int(5e6), "threads": 1} def tearDown(self): - test_helpers.remove_output_files(list_of_names=["_AbinsAdvanced"], directory=Path(self._cache_directory)) + self._tmpdir.cleanup() mtd.clear() def test_wrong_fwhm(self): @@ -67,7 +66,12 @@ def test_wrong_fwhm(self): for fwhm in bad_fwhm_values: abins.parameters.instruments["fwhm"] = fwhm self.assertRaisesRegex( - RuntimeError, "Invalid value of fwhm", Abins, VibrationalOrPhononFile=self._Si2 + ".phonon", OutputWorkspace=self._wrk_name + RuntimeError, + "Invalid value of fwhm", + Abins, + VibrationalOrPhononFile=self._Si2 + ".phonon", + OutputWorkspace=self._wrk_name, + CacheDirectory=self._cache_directory, ) # Tests for TOSCA parameters diff --git a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py index e81d0f1087f6..54df5ec9e0ea 100644 --- a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py +++ b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py @@ -4,11 +4,12 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + +from tempfile import TemporaryDirectory import unittest + from numpy.testing import assert_array_almost_equal from mantid.simpleapi import mtd, Abins, Scale, CompareWorkspaces, Load -import abins from abins.constants import ATOM_PREFIX, FUNDAMENTALS @@ -30,25 +31,12 @@ class AbinsBasicTest(unittest.TestCase): _workspace_name = "output_workspace" _tolerance = 0.0001 - from mantid.kernel import ConfigService - - _cache_directory = ConfigService.getString("defaultsave.directory") + def setUp(self): + self._tmpdir = TemporaryDirectory() + self._cache_directory = self._tmpdir.name def tearDown(self): - abins.test_helpers.remove_output_files( - list_of_names=[ - "explicit", - "default", - "total", - "squaricn_sum_Abins", - "squaricn_scale", - "benzene_exp", - "benzene_Abins", - "experimental", - "numbered", - ], - directory=self._cache_directory, - ) + self._tmpdir.cleanup() mtd.clear() def test_wrong_input(self): diff --git a/scripts/test/Abins/AbinsCalculateSPowderTest.py b/scripts/test/Abins/AbinsCalculateSPowderTest.py index 132d4fe57acc..1ae22042debf 100644 --- a/scripts/test/Abins/AbinsCalculateSPowderTest.py +++ b/scripts/test/Abins/AbinsCalculateSPowderTest.py @@ -51,11 +51,8 @@ def setUp(self): ) def tearDown(self): - from mantid.kernel import ConfigService + self._temporary_directory.cleanup() - abins.test_helpers.remove_output_files( - list_of_names=["_CalculateSPowder"], directory=ConfigService.getString("defaultsave.directory") - ) abins.parameters.performance["threads"] = self.default_threads abins.parameters.instruments.update(self._instruments_defaults) diff --git a/scripts/test/Abins/AbinsPowderCalculatorTest.py b/scripts/test/Abins/AbinsPowderCalculatorTest.py index 51503c5c7b0e..845d247c7893 100644 --- a/scripts/test/Abins/AbinsPowderCalculatorTest.py +++ b/scripts/test/Abins/AbinsPowderCalculatorTest.py @@ -6,6 +6,7 @@ # SPDX - License - Identifier: GPL - 3.0 + import json from pathlib import Path +from tempfile import TemporaryDirectory import unittest import numpy as np @@ -25,12 +26,11 @@ class PowderCalculatorTest(unittest.TestCase): # test input def setUp(self): - from mantid.kernel import ConfigService - - self._cache_directory = Path(ConfigService.getString("defaultsave.directory")) + self._tempdir = TemporaryDirectory() + self._cache_directory = Path(self._tempdir.name) def tearDown(self): - abins.test_helpers.remove_output_files(list_of_names=["CalculatePowder"], directory=self._cache_directory) + self._tempdir.cleanup() def test_wrong_input(self): full_path_filename = abins.test_helpers.find_file(filename=self._si2 + ".json") From ff9d56def4800ac72fb34d5f4e548dd9a351ed02 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Fri, 17 Jan 2025 12:25:15 +0000 Subject: [PATCH 09/19] Rely entirely on tempfile to cleanup Abins tests --- scripts/abins/test_helpers.py | 18 +----------------- scripts/test/Abins/AbinsDataTest.py | 15 +++++++++------ scripts/test/Abins/AbinsIOmoduleTest.py | 20 ++++++++++---------- scripts/test/Abins/AbinsLoadCASTEPTest.py | 5 ----- scripts/test/Abins/AbinsLoadCRYSTALTest.py | 5 ----- scripts/test/Abins/AbinsLoadDMOL3Test.py | 5 ----- scripts/test/Abins/AbinsLoadGAUSSIANTest.py | 7 +------ scripts/test/Abins/AbinsLoadJSONTest.py | 4 ---- scripts/test/Abins/AbinsLoadPhonopyTest.py | 4 ---- scripts/test/Abins/AbinsLoadVASPTest.py | 6 ------ 10 files changed, 21 insertions(+), 68 deletions(-) diff --git a/scripts/abins/test_helpers.py b/scripts/abins/test_helpers.py index f64bb72b73e6..02c5aafd8c9a 100644 --- a/scripts/abins/test_helpers.py +++ b/scripts/abins/test_helpers.py @@ -4,14 +4,12 @@ # NScD Oak Ridge National Laboratory, European Spallation Source, # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + -import os from pathlib import Path -from typing import Any, Dict, List, Mapping +from typing import Any, Dict, Mapping from unittest import TestCase import numpy as np from numpy.testing import assert_allclose -from pydantic import validate_call from abins.atomsdata import _AtomData from abins.kpointsdata import KpointData @@ -39,20 +37,6 @@ def find_file(filename: str, try_upcase_suffix: bool = True) -> str: raise ValueError(f"Could not find file '{filename}'") -@validate_call -def remove_output_files(list_of_names: List[str], directory: Path) -> None: - """Removes output files created during a test.""" - all_files = os.listdir(directory) - - for filename in all_files: - for name in list_of_names: - if name in filename: - full_path = directory / filename - if full_path.exists(): - os.remove(full_path) - break - - def dict_arrays_to_lists(mydict: Mapping[str, Any]) -> Dict[str, Any]: """Recursively convert numpy arrays in a nested dict to lists (i.e. valid JSON) diff --git a/scripts/test/Abins/AbinsDataTest.py b/scripts/test/Abins/AbinsDataTest.py index c4abe930dd5b..24af6d9ab09f 100644 --- a/scripts/test/Abins/AbinsDataTest.py +++ b/scripts/test/Abins/AbinsDataTest.py @@ -22,13 +22,15 @@ class TestAbinsData(unittest.TestCase): def setUp(self): - from mantid.kernel import ConfigService - - self.cache_directory = ConfigService.getString("defaultsave.directory") + self._tempdir = TemporaryDirectory() + self.cache_directory = Path(self._tempdir.name) self.mock_ad = MagicMock(spec=abins.AtomsData) self.mock_kpd = MagicMock(spec=abins.KpointsData) + def tearDown(self): + self._tempdir.cleanup() + def test_init_typeerror(self): with self.assertRaisesRegex(ValidationError, "Input should be an instance of KpointsData"): AbinsData(k_points_data=1, atoms_data=self.mock_ad) @@ -61,13 +63,14 @@ def get_formatted_data(self): class TestAbinsDataFromCalculation(unittest.TestCase): def setUp(self): - from mantid.kernel import ConfigService - - self.cache_directory = ConfigService.getString("defaultsave.directory") + self._tempdir = TemporaryDirectory() + self.cache_directory = Path(self._tempdir.name) all_loaders["DUMMYLOADER"] = DummyLoader def tearDown(self): + self._tempdir.cleanup() + del all_loaders["DUMMYLOADER"] def test_with_dummy_loader(self): diff --git a/scripts/test/Abins/AbinsIOmoduleTest.py b/scripts/test/Abins/AbinsIOmoduleTest.py index 9bee9e0fecc8..0341408b9c1a 100644 --- a/scripts/test/Abins/AbinsIOmoduleTest.py +++ b/scripts/test/Abins/AbinsIOmoduleTest.py @@ -5,24 +5,24 @@ # Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS # SPDX - License - Identifier: GPL - 3.0 + from pathlib import Path +from tempfile import TemporaryDirectory import unittest import numpy as np from pydantic import ValidationError -from abins import IO, test_helpers +from abins import IO class IOTest(unittest.TestCase): def setUp(self): - from mantid.kernel import ConfigService - - self._cache_directory = Path(ConfigService.getString("defaultsave.directory")) + self._tmpdir = TemporaryDirectory() + self.cache_directory = Path(self._tmpdir.name) def tearDown(self): - test_helpers.remove_output_files(list_of_names=["Cars", "temphgfrt"], directory=self._cache_directory) + self._tmpdir.cleanup() def _save_stuff(self): - saver = IO(input_filename="Cars.foo", group_name="Volksvagen", cache_directory=self._cache_directory) + saver = IO(input_filename="Cars.foo", group_name="Volksvagen", cache_directory=self.cache_directory) # add some attributes saver.add_attribute("Fuel", 100) @@ -48,13 +48,13 @@ def _save_stuff(self): saver.save() def _add_wrong_attribute(self): - poor_saver = IO(input_filename="BadCars.foo", group_name="Volksvagen", cache_directory=self._cache_directory) + poor_saver = IO(input_filename="BadCars.foo", group_name="Volksvagen", cache_directory=self.cache_directory) with self.assertRaisesRegex(ValidationError, "Input should be an instance of int64"): poor_saver.add_attribute("BadPassengers", np.array([4, 5], dtype=np.int64)) def _save_wrong_dataset(self): - poor_saver = IO(input_filename="BadCars.foo", group_name="Volksvagen", cache_directory=self._cache_directory) + poor_saver = IO(input_filename="BadCars.foo", group_name="Volksvagen", cache_directory=self.cache_directory) poor_saver.add_data("BadPassengers", 4) self.assertRaises(TypeError, poor_saver.save) @@ -68,7 +68,7 @@ def _wrong_groupname(self): self.assertRaises(ValueError, IO, input_filename="goodfile", group_name=1) def _wrong_file(self): - poor_loader = IO(input_filename="bumCars", group_name="nice_group", cache_directory=self._cache_directory) + poor_loader = IO(input_filename="bumCars", group_name="nice_group", cache_directory=self.cache_directory) self.assertRaises(IOError, poor_loader.load, list_of_attributes="one_attribute") def _loading_attributes(self): @@ -119,7 +119,7 @@ def runTest(self): self._add_wrong_attribute() self._save_wrong_dataset() - self.loader = IO(input_filename="Cars.foo", group_name="Volksvagen", cache_directory=self._cache_directory) + self.loader = IO(input_filename="Cars.foo", group_name="Volksvagen", cache_directory=self.cache_directory) self._wrong_filename_type() self._empty_filename() diff --git a/scripts/test/Abins/AbinsLoadCASTEPTest.py b/scripts/test/Abins/AbinsLoadCASTEPTest.py index 08943d35db0c..5f7ebb4e4781 100644 --- a/scripts/test/Abins/AbinsLoadCASTEPTest.py +++ b/scripts/test/Abins/AbinsLoadCASTEPTest.py @@ -48,11 +48,6 @@ def test_non_existing_file(self): # noinspection PyUnusedLocal CASTEPLoader(input_ab_initio_filename=1) - def tearDown(self): - from mantid.kernel import ConfigService - - abins.test_helpers.remove_output_files(list_of_names=["_LoadCASTEP"], directory=ConfigService.getString("defaultsave.directory")) - # *************************** USE CASES ******************************************** # =================================================================================== # | Use case: Gamma point calculation and sum correction enabled during calculations| diff --git a/scripts/test/Abins/AbinsLoadCRYSTALTest.py b/scripts/test/Abins/AbinsLoadCRYSTALTest.py index 7fbd76678e66..24fdf699665d 100644 --- a/scripts/test/Abins/AbinsLoadCRYSTALTest.py +++ b/scripts/test/Abins/AbinsLoadCRYSTALTest.py @@ -11,11 +11,6 @@ class AbinsLoadCRYSTALTest(unittest.TestCase, abins.input.Tester): - def tearDown(self): - from mantid.kernel import ConfigService - - abins.test_helpers.remove_output_files(list_of_names=["_LoadCRYSTAL"], directory=ConfigService.getString("defaultsave.directory")) - # *************************** USE CASES ********************************************* # =================================================================================== # | Use cases: Gamma point calculation for CRYSTAL | diff --git a/scripts/test/Abins/AbinsLoadDMOL3Test.py b/scripts/test/Abins/AbinsLoadDMOL3Test.py index 981f13758e44..5ba3c9490375 100644 --- a/scripts/test/Abins/AbinsLoadDMOL3Test.py +++ b/scripts/test/Abins/AbinsLoadDMOL3Test.py @@ -14,11 +14,6 @@ class AbinsLoadDMOL3Test(unittest.TestCase, abins.input.Tester): """Check entire input files against expected outputs""" - def tearDown(self): - from mantid.kernel import ConfigService - - abins.test_helpers.remove_output_files(list_of_names=["_LoadDMOL3"], directory=ConfigService.getString("defaultsave.directory")) - _gamma_dmol3 = "LTA_40_O2_LoadDMOL3" _gamma_no_h_dmol3 = "Na2SiF6_LoadDMOL3" _molecule_dmol3 = "methane_LoadDMOL3" diff --git a/scripts/test/Abins/AbinsLoadGAUSSIANTest.py b/scripts/test/Abins/AbinsLoadGAUSSIANTest.py index f955b87dddc9..984b9f9a88f7 100644 --- a/scripts/test/Abins/AbinsLoadGAUSSIANTest.py +++ b/scripts/test/Abins/AbinsLoadGAUSSIANTest.py @@ -11,12 +11,7 @@ class AbinsLoadGAUSSIANTest(unittest.TestCase, abins.input.Tester): - def tearDown(self): - from mantid.kernel import ConfigService - - abins.test_helpers.remove_output_files(list_of_names=["_LoadGAUSSIAN"], directory=ConfigService.getString("defaultsave.directory")) - - # *************************** USE CASES ******************************************** + # *************************** USE CASES ******************************************** # =================================================================================== # | Use cases: molecular calculation for GAUSSIAN03 Hartree Fock, Unix | diff --git a/scripts/test/Abins/AbinsLoadJSONTest.py b/scripts/test/Abins/AbinsLoadJSONTest.py index 98eeff503594..35da36b8e602 100644 --- a/scripts/test/Abins/AbinsLoadJSONTest.py +++ b/scripts/test/Abins/AbinsLoadJSONTest.py @@ -17,10 +17,6 @@ def setUp(self): abins.parameters.sampling["force_constants"]["qpt_cutoff"] = 4.0 def tearDown(self): - from mantid.kernel import ConfigService - - abins.test_helpers.remove_output_files(list_of_names=["_LoadJSON"], directory=ConfigService.getString("defaultsave.directory")) - abins.parameters.sampling["force_constants"]["qpt_cutoff"] = self.default_cutoff def test_json_1(self): diff --git a/scripts/test/Abins/AbinsLoadPhonopyTest.py b/scripts/test/Abins/AbinsLoadPhonopyTest.py index 11a0f2f315ab..1cf60a53853c 100644 --- a/scripts/test/Abins/AbinsLoadPhonopyTest.py +++ b/scripts/test/Abins/AbinsLoadPhonopyTest.py @@ -21,10 +21,6 @@ def setUp(self): abins.parameters.sampling["force_constants"]["qpt_cutoff"] = 4.0 def tearDown(self): - from mantid.kernel import ConfigService - - abins.test_helpers.remove_output_files(list_of_names=["_LoadPhonopy"], directory=ConfigService.getString("defaultsave.directory")) - abins.parameters.sampling["force_constants"]["qpt_cutoff"] = self.default_cutoff def test_non_existing_file(self): diff --git a/scripts/test/Abins/AbinsLoadVASPTest.py b/scripts/test/Abins/AbinsLoadVASPTest.py index a677a4e00fad..b9d40341bee2 100644 --- a/scripts/test/Abins/AbinsLoadVASPTest.py +++ b/scripts/test/Abins/AbinsLoadVASPTest.py @@ -13,12 +13,6 @@ class AbinsLoadVASPTest(unittest.TestCase, abins.input.Tester): - # def tearDown(self): - # # Remove cache files - # from mantid.kernel import ConfigService - - # abins.test_helpers.remove_output_files(list_of_names=["_LoadVASP"], directory=ConfigService.getString("defaultsave.directory")) - def test_non_existing_file(self): with TemporaryDirectory() as tmpdir: with self.assertRaises(IOError): From 8c4079fcce6a14b7e3b60766d9ad422b54faf29d Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Fri, 17 Jan 2025 12:28:12 +0000 Subject: [PATCH 10/19] More consistent attribute names --- scripts/test/Abins/AbinsCalculateSPowderTest.py | 14 +++++++------- scripts/test/Abins/AbinsIOmoduleTest.py | 6 +++--- scripts/test/Abins/AbinsPowderCalculatorTest.py | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/scripts/test/Abins/AbinsCalculateSPowderTest.py b/scripts/test/Abins/AbinsCalculateSPowderTest.py index 1ae22042debf..594749eecd5a 100644 --- a/scripts/test/Abins/AbinsCalculateSPowderTest.py +++ b/scripts/test/Abins/AbinsCalculateSPowderTest.py @@ -39,7 +39,7 @@ def setUp(self): abins.parameters.performance["threads"] = 1 self._instruments_defaults = deepcopy(abins.parameters.instruments) self._temporary_directory = TemporaryDirectory() - self._cache_directory = Path(self._temporary_directory.name) + self.cache_directory = Path(self._temporary_directory.name) self.default_calculator_kwargs = dict( temperature=self._temperature, @@ -47,7 +47,7 @@ def setUp(self): sample_form=self._sample_form, quantum_order_num=self._order_event, autoconvolution_max=0, - cache_directory=self._cache_directory, + cache_directory=self.cache_directory, ) def tearDown(self): @@ -71,7 +71,7 @@ def test_invalid_input(self): abins_data=good_data, instrument=self._instrument, quantum_order_num=self._order_event, - cache_directory=self._cache_directory, + cache_directory=self.cache_directory, ) # invalid temperature @@ -83,7 +83,7 @@ def test_invalid_input(self): abins_data=good_data, instrument=self._instrument, quantum_order_num=self._order_event, - cache_directory=self._cache_directory, + cache_directory=self.cache_directory, ) # invalid sample @@ -95,7 +95,7 @@ def test_invalid_input(self): abins_data=good_data, instrument=self._instrument, quantum_order_num=self._order_event, - cache_directory=self._cache_directory, + cache_directory=self.cache_directory, ) # invalid abins data: content of abins data instead of object abins_data @@ -107,7 +107,7 @@ def test_invalid_input(self): abins_data=good_data.extract(), instrument=self._instrument, quantum_order_num=self._order_event, - cache_directory=self._cache_directory, + cache_directory=self.cache_directory, ) # invalid instrument @@ -119,7 +119,7 @@ def test_invalid_input(self): abins_data=good_data, instrument="INSTRUMENT", quantum_order_num=self._order_event, - cache_directory=self._cache_directory, + cache_directory=self.cache_directory, ) def test_1d_order1(self): diff --git a/scripts/test/Abins/AbinsIOmoduleTest.py b/scripts/test/Abins/AbinsIOmoduleTest.py index 0341408b9c1a..58ea00a5f121 100644 --- a/scripts/test/Abins/AbinsIOmoduleTest.py +++ b/scripts/test/Abins/AbinsIOmoduleTest.py @@ -15,11 +15,11 @@ class IOTest(unittest.TestCase): def setUp(self): - self._tmpdir = TemporaryDirectory() - self.cache_directory = Path(self._tmpdir.name) + self.tempdir = TemporaryDirectory() + self.cache_directory = Path(self.tempdir.name) def tearDown(self): - self._tmpdir.cleanup() + self.tempdir.cleanup() def _save_stuff(self): saver = IO(input_filename="Cars.foo", group_name="Volksvagen", cache_directory=self.cache_directory) diff --git a/scripts/test/Abins/AbinsPowderCalculatorTest.py b/scripts/test/Abins/AbinsPowderCalculatorTest.py index 845d247c7893..d1121cd5cd01 100644 --- a/scripts/test/Abins/AbinsPowderCalculatorTest.py +++ b/scripts/test/Abins/AbinsPowderCalculatorTest.py @@ -27,7 +27,7 @@ class PowderCalculatorTest(unittest.TestCase): # test input def setUp(self): self._tempdir = TemporaryDirectory() - self._cache_directory = Path(self._tempdir.name) + self.cache_directory = Path(self._tempdir.name) def tearDown(self): self._tempdir.cleanup() @@ -59,7 +59,7 @@ def _good_case(self, name=None): ref_data = self._get_ref_data(filename=abins_data_filename) good_tester = abins.PowderCalculator( - filename=abins_data_filename, abins_data=ref_data["DFT"], temperature=300.0, cache_directory=self._cache_directory + filename=abins_data_filename, abins_data=ref_data["DFT"], temperature=300.0, cache_directory=self.cache_directory ) calculated_data = good_tester.calculate_data().extract() @@ -73,7 +73,7 @@ def _good_case(self, name=None): # check if loading powder data is correct new_tester = abins.PowderCalculator( - filename=abins_data_filename, abins_data=ref_data["DFT"], temperature=300.0, cache_directory=self._cache_directory + filename=abins_data_filename, abins_data=ref_data["DFT"], temperature=300.0, cache_directory=self.cache_directory ) loaded_data = new_tester.load_formatted_data().extract() @@ -101,7 +101,7 @@ def _write_data(self, name=None): abins_data = AbinsData.from_dict(json.load(fp)) powder = abins.PowderCalculator( - filename=abins_data_filename, abins_data=abins_data, temperature=300.0, cache_directory=self._cache_directory + filename=abins_data_filename, abins_data=abins_data, temperature=300.0, cache_directory=self.cache_directory ).calculate_data() powder_dict = abins.test_helpers.dict_arrays_to_lists(powder.extract()) From 32321ba5cb2780424cfd34bd2dca9d5a6b86e572 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Fri, 17 Jan 2025 12:55:02 +0000 Subject: [PATCH 11/19] Make cache_directory argument mandatory --- scripts/abins/io.py | 2 +- scripts/abins/powdercalculator.py | 2 +- scripts/abins/scalculatorfactory.py | 2 +- scripts/abins/spowdersemiempiricalcalculator.py | 2 +- scripts/test/Abins/AbinsPowderCalculatorTest.py | 17 ++++++++++++++--- 5 files changed, 18 insertions(+), 7 deletions(-) diff --git a/scripts/abins/io.py b/scripts/abins/io.py index 7c58a95d2874..79737bd4558c 100644 --- a/scripts/abins/io.py +++ b/scripts/abins/io.py @@ -35,7 +35,7 @@ class IO(BaseModel): setting: str = "" autoconvolution: int = 10 temperature: float = None - cache_directory: Path | None = None + cache_directory: Path @staticmethod def _dir_is_not_writeable(directory: str): diff --git a/scripts/abins/powdercalculator.py b/scripts/abins/powdercalculator.py index 8dc7eff9076b..f3b746368796 100644 --- a/scripts/abins/powdercalculator.py +++ b/scripts/abins/powdercalculator.py @@ -19,7 +19,7 @@ class PowderCalculator: Class for calculating powder data. """ - def __init__(self, *, filename: str, abins_data: abins.AbinsData, temperature: float, cache_directory: Path | None = None) -> None: + def __init__(self, *, filename: str, abins_data: abins.AbinsData, temperature: float, cache_directory: Path) -> None: """ :param filename: name of input DFT filename :param abins_data: object of type AbinsData with data from input DFT file diff --git a/scripts/abins/scalculatorfactory.py b/scripts/abins/scalculatorfactory.py index 7f6c4ae960cb..ec4f863d137f 100644 --- a/scripts/abins/scalculatorfactory.py +++ b/scripts/abins/scalculatorfactory.py @@ -30,7 +30,7 @@ def init( instrument: Instrument, quantum_order_num: int, autoconvolution_max: int = 0, - cache_directory: Path | None = None, + cache_directory: Path, ): """ :param filename: name of input DFT file (CASTEP: foo.phonon) diff --git a/scripts/abins/spowdersemiempiricalcalculator.py b/scripts/abins/spowdersemiempiricalcalculator.py index bea91a2fa43e..e466ed33798e 100644 --- a/scripts/abins/spowdersemiempiricalcalculator.py +++ b/scripts/abins/spowdersemiempiricalcalculator.py @@ -35,7 +35,7 @@ def __init__( instrument: Instrument, quantum_order_num: int = Field(ge=1, le=2), autoconvolution_max: int = 0, - cache_directory: Path | None = None, + cache_directory: Path, ) -> None: """ :param filename: name of input DFT file (CASTEP: foo.phonon). This is only used for caching, the file will not be read. diff --git a/scripts/test/Abins/AbinsPowderCalculatorTest.py b/scripts/test/Abins/AbinsPowderCalculatorTest.py index d1121cd5cd01..29ac0a3a775e 100644 --- a/scripts/test/Abins/AbinsPowderCalculatorTest.py +++ b/scripts/test/Abins/AbinsPowderCalculatorTest.py @@ -38,14 +38,25 @@ def test_wrong_input(self): abins_data = AbinsData.from_dict(json.load(fp)) # wrong filename - self.assertRaises(ValueError, PowderCalculator, filename=1, abins_data=abins_data, temperature=300.0) + self.assertRaises( + ValueError, PowderCalculator, filename=1, abins_data=abins_data, temperature=300.0, cache_directory=self.cache_directory + ) # data from object of type AtomsData instead of object of type AbinsData wrong_type_data = abins_data.get_atoms_data() - self.assertRaises(ValueError, PowderCalculator, filename=full_path_filename, abins_data=wrong_type_data, temperature=300.0) + self.assertRaises( + ValueError, + PowderCalculator, + filename=full_path_filename, + abins_data=wrong_type_data, + temperature=300.0, + cache_directory=self.cache_directory, + ) # Missing Temperature - self.assertRaises(TypeError, PowderCalculator, filename=full_path_filename, abins_data=abins_data) + self.assertRaises( + TypeError, PowderCalculator, filename=full_path_filename, abins_data=abins_data, cache_directory=self.cache_directory + ) # main test def test_good_case(self): From 031ca57c12788ea50df56fbb893c41547e26148c Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Fri, 17 Jan 2025 12:57:04 +0000 Subject: [PATCH 12/19] More consistent attribute names --- .../plugins/algorithms/AbinsAdvancedParametersTest.py | 6 +++--- .../test/python/plugins/algorithms/AbinsBasicTest.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py index 812cb07d94c8..38ddde661e69 100644 --- a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py +++ b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsAdvancedParametersTest.py @@ -23,8 +23,8 @@ def setUp(self): # set up input for Abins self._Si2 = "Si2-sc_AbinsAdvancedParameters" self._wrk_name = self._Si2 + "_ref" - self._tmpdir = TemporaryDirectory() - self._cache_directory = self._tmpdir.name + self.tempdir = TemporaryDirectory() + self._cache_directory = self.tempdir.name # before each test set abins.parameters to default values abins.parameters.instruments = { @@ -55,7 +55,7 @@ def setUp(self): abins.parameters.performance = {"optimal_size": int(5e6), "threads": 1} def tearDown(self): - self._tmpdir.cleanup() + self.tempdir.cleanup() mtd.clear() def test_wrong_fwhm(self): diff --git a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py index 54df5ec9e0ea..f1562a19b95b 100644 --- a/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py +++ b/Framework/PythonInterface/test/python/plugins/algorithms/AbinsBasicTest.py @@ -32,11 +32,11 @@ class AbinsBasicTest(unittest.TestCase): _tolerance = 0.0001 def setUp(self): - self._tmpdir = TemporaryDirectory() - self._cache_directory = self._tmpdir.name + self.tempdir = TemporaryDirectory() + self._cache_directory = self.tempdir.name def tearDown(self): - self._tmpdir.cleanup() + self.tempdir.cleanup() mtd.clear() def test_wrong_input(self): From 6981fcbbd694b99b81f16053469b805f375a5808 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 17 Jan 2025 13:09:40 +0000 Subject: [PATCH 13/19] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- scripts/abins/io.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/abins/io.py b/scripts/abins/io.py index 79737bd4558c..533a90a4b0ec 100644 --- a/scripts/abins/io.py +++ b/scripts/abins/io.py @@ -86,8 +86,7 @@ def model_post_init(self, __context): if self._dir_is_not_writeable(str(self.cache_directory)): raise Exception( - f"Could not write a file to the cache directory {self.cache_directory}. " - "Please check this location is reasonable." + f"Could not write a file to the cache directory {self.cache_directory}. Please check this location is reasonable." ) self._hdf_filename = str(self.cache_directory / f"{core_name}.hdf5") From 2afee748e032778d8deb3a0f8cb340c9b9226a66 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Mon, 20 Jan 2025 10:58:15 +0000 Subject: [PATCH 14/19] Add CacheDirectory to Abins-1 doctest Abins2D will still break. I'm not really happy with this "fix" because there _should_ be a default argument. --- docs/source/algorithms/Abins-v1.rst | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/source/algorithms/Abins-v1.rst b/docs/source/algorithms/Abins-v1.rst index 0bffb675f1a6..c46b104c7a5f 100644 --- a/docs/source/algorithms/Abins-v1.rst +++ b/docs/source/algorithms/Abins-v1.rst @@ -61,7 +61,7 @@ Usage .. testcode:: AbinsCastepSimple benzene_wrk = Abins(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon", - QuantumOrderEventsNumber="1") + QuantumOrderEventsNumber="1", CacheDirectory="/tmp") for name in benzene_wrk.getNames(): @@ -79,13 +79,13 @@ Output: .. testcleanup:: AbinsCastepSimple import os - os.remove("benzene.hdf5") + os.remove("/tmp/benzene.hdf5") **Example - loading CRYSTAL phonon data:** .. testcode:: AbinsCrystalSimple - wrk=Abins(AbInitioProgram="CRYSTAL", VibrationalOrPhononFile="b3lyp.out", QuantumOrderEventsNumber="1") + wrk=Abins(AbInitioProgram="CRYSTAL", VibrationalOrPhononFile="b3lyp.out", QuantumOrderEventsNumber="1", CacheDirectory="/tmp") for name in wrk.getNames(): print(name) @@ -108,7 +108,7 @@ Output: .. testcleanup:: AbinsCrystalSimple import os - os.remove("b3lyp.hdf5") + os.remove("/tmp/b3lyp.hdf5") **Example - calling AbINS with more arguments:** @@ -117,7 +117,8 @@ Output: wrk_verbose=Abins(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon", ExperimentalFile="benzene_experimental.dat", TemperatureInKelvin=10, BinWidthInWavenumber=1.0, SampleForm="Powder", Instrument="TOSCA", - Atoms="H, atom1, atom2", SumContributions=True, QuantumOrderEventsNumber="1", ScaleByCrossSection="Incoherent") + Atoms="H, atom1, atom2", SumContributions=True, QuantumOrderEventsNumber="1", ScaleByCrossSection="Incoherent", + CacheDirectory="/tmp") for name in wrk_verbose.getNames(): print(name) @@ -138,7 +139,7 @@ Output: .. testcleanup:: AbinsexplicitParameters import os - os.remove("benzene.hdf5") + os.remove("/tmp/benzene.hdf5") .. categories:: From 067bd9e55d4d01c0c9205469b5894244b511d560 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Mon, 20 Jan 2025 14:33:01 +0000 Subject: [PATCH 15/19] Fix Abins2D doctest (but a bit clunky) As with Abins-v1, we fix the doctest by - providing a CacheDirectory argument to the Algorithm calls - setting a corresponding cleanup path The problems with this approach are: - it's ugly and distracting - it _should_ be unnecessary, there is a default value and it works when calling from GUI - /tmp probably doesn't work on Windows --- docs/source/algorithms/Abins2D-v1.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/source/algorithms/Abins2D-v1.rst b/docs/source/algorithms/Abins2D-v1.rst index 8d1f55e7dd05..0b8332182386 100644 --- a/docs/source/algorithms/Abins2D-v1.rst +++ b/docs/source/algorithms/Abins2D-v1.rst @@ -58,7 +58,7 @@ A minimal example, relying heavily on default parameters: .. testcode:: Abins2DCastepSimple - benzene_wrk = Abins2D(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon") + benzene_wrk = Abins2D(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon", CacheDirectory="/tmp/") for name in benzene_wrk.getNames(): @@ -76,7 +76,7 @@ Output: (note that only the fundamental excitations are included) .. testcleanup:: Abins2DCastepSimple import os - os.remove("benzene.hdf5") + os.remove("/tmp/benzene.hdf5") **Example - using more arguments:** @@ -94,7 +94,7 @@ approximation may not be the appropriate tool.) TemperatureInKelvin=10, Instrument="MAPS", Atoms="H, atom1, atom2", SumContributions=True, QuantumOrderEventsNumber="2", Autoconvolution=True, - ScaleByCrossSection="Total") + ScaleByCrossSection="Total", CacheDirectory="/tmp") print(f"Created {wrk_verbose.size()} workspaces") print(f"including {wrk_verbose[12].name()}") @@ -109,7 +109,7 @@ Output: .. testcleanup:: Abins2DExplicitParameters import os - os.remove("benzene.hdf5") + os.remove("/tmp/benzene.hdf5") .. categories:: From a772d3a90ad902c538c3590254891a4c6abfda94 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Wed, 22 Jan 2025 10:04:56 +0000 Subject: [PATCH 16/19] Add release note --- .../release/v6.13.0/Indirect/Algorithms/New_features/38620.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/source/release/v6.13.0/Indirect/Algorithms/New_features/38620.rst diff --git a/docs/source/release/v6.13.0/Indirect/Algorithms/New_features/38620.rst b/docs/source/release/v6.13.0/Indirect/Algorithms/New_features/38620.rst new file mode 100644 index 000000000000..8802b96d2903 --- /dev/null +++ b/docs/source/release/v6.13.0/Indirect/Algorithms/New_features/38620.rst @@ -0,0 +1 @@ +- A "CacheDirectory" parameter has been added to the Abins and Abins2D algorithms. This allows an explicit cache directory to be set independently for each calculation; the previous behaviour (to use the User Save Directory) remains available as the default value. From fcddc704baf0ace0c14b78d23d8e9cef65326c6f Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Mon, 10 Feb 2025 16:35:25 +0000 Subject: [PATCH 17/19] Doctests: default parameter for CacheDirectory seems to work! It has been a bit of a headache getting these to work nicely, but it may have been something to do with spacing between RST lines. There seems to be a little API change at work here: the default cache director is now always the default save directory. Previously, it seems that command-line runs of Mantid would sometimes use the current directory from which Mantid was launched. That may be a headscratcher or even breaking for certain user scripts. In such workflows it would be highly recommended to set an explicit directory. --- docs/source/algorithms/Abins-v1.rst | 43 ++++++++++++++++----------- docs/source/algorithms/Abins2D-v1.rst | 16 +++++++--- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/docs/source/algorithms/Abins-v1.rst b/docs/source/algorithms/Abins-v1.rst index c46b104c7a5f..0169eb73f38d 100644 --- a/docs/source/algorithms/Abins-v1.rst +++ b/docs/source/algorithms/Abins-v1.rst @@ -61,8 +61,7 @@ Usage .. testcode:: AbinsCastepSimple benzene_wrk = Abins(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon", - QuantumOrderEventsNumber="1", CacheDirectory="/tmp") - + QuantumOrderEventsNumber="1") for name in benzene_wrk.getNames(): print(name) @@ -79,13 +78,18 @@ Output: .. testcleanup:: AbinsCastepSimple import os - os.remove("/tmp/benzene.hdf5") + from mantid.kernel import ConfigService + + savedir = ConfigService.getString("defaultsave.directory") + + os.remove(os.path.join(savedir, "benzene.hdf5")) + **Example - loading CRYSTAL phonon data:** .. testcode:: AbinsCrystalSimple - wrk=Abins(AbInitioProgram="CRYSTAL", VibrationalOrPhononFile="b3lyp.out", QuantumOrderEventsNumber="1", CacheDirectory="/tmp") + wrk=Abins(AbInitioProgram="CRYSTAL", VibrationalOrPhononFile="b3lyp.out", QuantumOrderEventsNumber="1") for name in wrk.getNames(): print(name) @@ -108,24 +112,34 @@ Output: .. testcleanup:: AbinsCrystalSimple import os - os.remove("/tmp/b3lyp.hdf5") + from mantid.kernel import ConfigService + + savedir = ConfigService.getString("defaultsave.directory") + + os.remove(os.path.join(savedir, "b3lyp.hdf5")) **Example - calling AbINS with more arguments:** -.. testcode:: AbinsexplicitParameters +Here the cache file is directed to a temporary directory and will be cleaned up automatically. - wrk_verbose=Abins(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon", - ExperimentalFile="benzene_experimental.dat", - TemperatureInKelvin=10, BinWidthInWavenumber=1.0, SampleForm="Powder", Instrument="TOSCA", - Atoms="H, atom1, atom2", SumContributions=True, QuantumOrderEventsNumber="1", ScaleByCrossSection="Incoherent", - CacheDirectory="/tmp") +.. testcode:: AbinsExplicitParameters + + from tempfile import TemporaryDirectory + + with TemporaryDirectory() as tmp_dir: + + wrk_verbose=Abins(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon", + ExperimentalFile="benzene_experimental.dat", + TemperatureInKelvin=10, BinWidthInWavenumber=1.0, SampleForm="Powder", Instrument="TOSCA", + Atoms="H, atom1, atom2", SumContributions=True, QuantumOrderEventsNumber="1", ScaleByCrossSection="Incoherent", + CacheDirectory=tmp_dir) for name in wrk_verbose.getNames(): print(name) Output: -.. testoutput:: AbinsexplicitParameters +.. testoutput:: AbinsExplicitParameters experimental_wrk wrk_verbose_total @@ -136,11 +150,6 @@ Output: wrk_verbose_atom_2_total wrk_verbose_atom_2 -.. testcleanup:: AbinsexplicitParameters - - import os - os.remove("/tmp/benzene.hdf5") - .. categories:: .. sourcelink:: diff --git a/docs/source/algorithms/Abins2D-v1.rst b/docs/source/algorithms/Abins2D-v1.rst index 0b8332182386..2a8773105c7a 100644 --- a/docs/source/algorithms/Abins2D-v1.rst +++ b/docs/source/algorithms/Abins2D-v1.rst @@ -58,7 +58,7 @@ A minimal example, relying heavily on default parameters: .. testcode:: Abins2DCastepSimple - benzene_wrk = Abins2D(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon", CacheDirectory="/tmp/") + benzene_wrk = Abins2D(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon") for name in benzene_wrk.getNames(): @@ -76,7 +76,11 @@ Output: (note that only the fundamental excitations are included) .. testcleanup:: Abins2DCastepSimple import os - os.remove("/tmp/benzene.hdf5") + from mantid.kernel import ConfigService + + savedir = ConfigService.getString("defaultsave.directory") + + os.remove(os.path.join(savedir, "benzene.hdf5")) **Example - using more arguments:** @@ -94,7 +98,7 @@ approximation may not be the appropriate tool.) TemperatureInKelvin=10, Instrument="MAPS", Atoms="H, atom1, atom2", SumContributions=True, QuantumOrderEventsNumber="2", Autoconvolution=True, - ScaleByCrossSection="Total", CacheDirectory="/tmp") + ScaleByCrossSection="Total") print(f"Created {wrk_verbose.size()} workspaces") print(f"including {wrk_verbose[12].name()}") @@ -109,7 +113,11 @@ Output: .. testcleanup:: Abins2DExplicitParameters import os - os.remove("/tmp/benzene.hdf5") + from mantid.kernel import ConfigService + + savedir = ConfigService.getString("defaultsave.directory") + + os.remove(os.path.join(savedir, "benzene.hdf5")) .. categories:: From 5e889bfa223282c4df7083d597ff51d9dcd664bc Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Fri, 14 Feb 2025 10:05:10 +0000 Subject: [PATCH 18/19] Rework defaultsave / cleanup logic for Abins doctests --- docs/source/algorithms/Abins-v1.rst | 33 ++++++++++++++++-------- docs/source/algorithms/Abins2D-v1.rst | 36 ++++++++++++++++++--------- 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/docs/source/algorithms/Abins-v1.rst b/docs/source/algorithms/Abins-v1.rst index 0169eb73f38d..d15e62304751 100644 --- a/docs/source/algorithms/Abins-v1.rst +++ b/docs/source/algorithms/Abins-v1.rst @@ -58,6 +58,16 @@ Usage **Example - loading CASTEP phonon data:** +.. testsetup:: AbinsCastepSimple + + from tempfile import TemporaryDirectory + from mantid.kernel import ConfigService + + test_dir = TemporaryDirectory() + + initial_defaultsave = ConfigService.getString("defaultsave.directory") + ConfigService.setString("defaultsave.directory", test_dir.name) + .. testcode:: AbinsCastepSimple benzene_wrk = Abins(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon", @@ -77,15 +87,20 @@ Output: .. testcleanup:: AbinsCastepSimple - import os - from mantid.kernel import ConfigService + ConfigService.setString("defaultsave.directory", initial_defaultsave) + test_dir.cleanup() - savedir = ConfigService.getString("defaultsave.directory") +**Example - loading CRYSTAL phonon data:** - os.remove(os.path.join(savedir, "benzene.hdf5")) +.. testsetup:: AbinsCrystalSimple + from tempfile import TemporaryDirectory + from mantid.kernel import ConfigService -**Example - loading CRYSTAL phonon data:** + test_dir = TemporaryDirectory() + + initial_defaultsave = ConfigService.getString("defaultsave.directory") + ConfigService.setString("defaultsave.directory", test_dir.name) .. testcode:: AbinsCrystalSimple @@ -111,12 +126,8 @@ Output: .. testcleanup:: AbinsCrystalSimple - import os - from mantid.kernel import ConfigService - - savedir = ConfigService.getString("defaultsave.directory") - - os.remove(os.path.join(savedir, "b3lyp.hdf5")) + test_dir.cleanup() + ConfigService.setString("defaultsave.directory", initial_defaultsave) **Example - calling AbINS with more arguments:** diff --git a/docs/source/algorithms/Abins2D-v1.rst b/docs/source/algorithms/Abins2D-v1.rst index 2a8773105c7a..de22c1b1df53 100644 --- a/docs/source/algorithms/Abins2D-v1.rst +++ b/docs/source/algorithms/Abins2D-v1.rst @@ -56,6 +56,16 @@ Usage A minimal example, relying heavily on default parameters: +.. testsetup:: Abins2DCastepSimple + + from tempfile import TemporaryDirectory + from mantid.kernel import ConfigService + + test_dir = TemporaryDirectory() + + initial_defaultsave = ConfigService.getString("defaultsave.directory") + ConfigService.setString("defaultsave.directory", test_dir.name) + .. testcode:: Abins2DCastepSimple benzene_wrk = Abins2D(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon") @@ -75,12 +85,8 @@ Output: (note that only the fundamental excitations are included) .. testcleanup:: Abins2DCastepSimple - import os - from mantid.kernel import ConfigService - - savedir = ConfigService.getString("defaultsave.directory") - - os.remove(os.path.join(savedir, "benzene.hdf5")) + test_dir.cleanup() + ConfigService.setString("defaultsave.directory", initial_defaultsave) **Example - using more arguments:** @@ -92,6 +98,16 @@ weights can be added for a slight improvement to the predicted spectrum. (If the spectrum is dominated by coherent scattering, this approximation may not be the appropriate tool.) +.. testsetup:: Abins2DExplicitParameters + + from tempfile import TemporaryDirectory + from mantid.kernel import ConfigService + + test_dir = TemporaryDirectory() + + initial_defaultsave = ConfigService.getString("defaultsave.directory") + ConfigService.setString("defaultsave.directory", test_dir.name) + .. testcode:: Abins2DExplicitParameters wrk_verbose=Abins2D(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon", @@ -112,12 +128,8 @@ Output: .. testcleanup:: Abins2DExplicitParameters - import os - from mantid.kernel import ConfigService - - savedir = ConfigService.getString("defaultsave.directory") - - os.remove(os.path.join(savedir, "benzene.hdf5")) + test_dir.cleanup() + ConfigService.setString("defaultsave.directory", initial_defaultsave) .. categories:: From 352943f24fe1ce374a8222cb79993688a432a185 Mon Sep 17 00:00:00 2001 From: "Adam J. Jackson" Date: Fri, 14 Feb 2025 17:33:52 +0000 Subject: [PATCH 19/19] Further Abins doctest cleanup Use magic * to share test setup/cleanup logic across tests. Move cleanup to top of file and use comments so it is easier to see what is going on. --- docs/source/algorithms/Abins-v1.rst | 34 ++++++++++---------------- docs/source/algorithms/Abins2D-v1.rst | 35 +++++++++++---------------- 2 files changed, 27 insertions(+), 42 deletions(-) diff --git a/docs/source/algorithms/Abins-v1.rst b/docs/source/algorithms/Abins-v1.rst index d15e62304751..9e076029db9c 100644 --- a/docs/source/algorithms/Abins-v1.rst +++ b/docs/source/algorithms/Abins-v1.rst @@ -58,7 +58,11 @@ Usage **Example - loading CASTEP phonon data:** -.. testsetup:: AbinsCastepSimple +.. testsetup:: * + + # On CI defaultsave.directory may be set to an inappropriate + # value, causing the input validator to raise an error. + # Set it somewhere that is guaranteed to be suitable. from tempfile import TemporaryDirectory from mantid.kernel import ConfigService @@ -68,6 +72,14 @@ Usage initial_defaultsave = ConfigService.getString("defaultsave.directory") ConfigService.setString("defaultsave.directory", test_dir.name) +.. testcleanup:: * + + # Restore the original defaultsave.directory to avoid surprises + # when running doctests locally. + + test_dir.cleanup() + ConfigService.setString("defaultsave.directory", initial_defaultsave) + .. testcode:: AbinsCastepSimple benzene_wrk = Abins(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon", @@ -85,23 +97,8 @@ Output: benzene_wrk_H_total benzene_wrk_H -.. testcleanup:: AbinsCastepSimple - - ConfigService.setString("defaultsave.directory", initial_defaultsave) - test_dir.cleanup() - **Example - loading CRYSTAL phonon data:** -.. testsetup:: AbinsCrystalSimple - - from tempfile import TemporaryDirectory - from mantid.kernel import ConfigService - - test_dir = TemporaryDirectory() - - initial_defaultsave = ConfigService.getString("defaultsave.directory") - ConfigService.setString("defaultsave.directory", test_dir.name) - .. testcode:: AbinsCrystalSimple wrk=Abins(AbInitioProgram="CRYSTAL", VibrationalOrPhononFile="b3lyp.out", QuantumOrderEventsNumber="1") @@ -124,11 +121,6 @@ Output: wrk_O_total wrk_O -.. testcleanup:: AbinsCrystalSimple - - test_dir.cleanup() - ConfigService.setString("defaultsave.directory", initial_defaultsave) - **Example - calling AbINS with more arguments:** Here the cache file is directed to a temporary directory and will be cleaned up automatically. diff --git a/docs/source/algorithms/Abins2D-v1.rst b/docs/source/algorithms/Abins2D-v1.rst index de22c1b1df53..e10ddb4f409d 100644 --- a/docs/source/algorithms/Abins2D-v1.rst +++ b/docs/source/algorithms/Abins2D-v1.rst @@ -56,7 +56,11 @@ Usage A minimal example, relying heavily on default parameters: -.. testsetup:: Abins2DCastepSimple +.. testsetup:: * + + # On CI defaultsave.directory may be set to an inappropriate + # value, causing the input validator to raise an error. + # Set it somewhere that is guaranteed to be suitable. from tempfile import TemporaryDirectory from mantid.kernel import ConfigService @@ -66,6 +70,15 @@ A minimal example, relying heavily on default parameters: initial_defaultsave = ConfigService.getString("defaultsave.directory") ConfigService.setString("defaultsave.directory", test_dir.name) +.. testcleanup:: * + + # Restore the original defaultsave.directory to avoid surprises + # when running doctests locally. + + test_dir.cleanup() + ConfigService.setString("defaultsave.directory", initial_defaultsave) + + .. testcode:: Abins2DCastepSimple benzene_wrk = Abins2D(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon") @@ -83,11 +96,6 @@ Output: (note that only the fundamental excitations are included) benzene_wrk_H_total benzene_wrk_H -.. testcleanup:: Abins2DCastepSimple - - test_dir.cleanup() - ConfigService.setString("defaultsave.directory", initial_defaultsave) - **Example - using more arguments:** In practice we would usually select an instrument, incident energy, @@ -98,16 +106,6 @@ weights can be added for a slight improvement to the predicted spectrum. (If the spectrum is dominated by coherent scattering, this approximation may not be the appropriate tool.) -.. testsetup:: Abins2DExplicitParameters - - from tempfile import TemporaryDirectory - from mantid.kernel import ConfigService - - test_dir = TemporaryDirectory() - - initial_defaultsave = ConfigService.getString("defaultsave.directory") - ConfigService.setString("defaultsave.directory", test_dir.name) - .. testcode:: Abins2DExplicitParameters wrk_verbose=Abins2D(AbInitioProgram="CASTEP", VibrationalOrPhononFile="benzene.phonon", @@ -126,11 +124,6 @@ Output: Created 34 workspaces including wrk_verbose_atom_1_total -.. testcleanup:: Abins2DExplicitParameters - - test_dir.cleanup() - ConfigService.setString("defaultsave.directory", initial_defaultsave) - .. categories:: .. sourcelink::