From 336f8f3584b35ee7421d481ddbd3371b29876bf1 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Mon, 22 Sep 2025 15:27:27 +0100 Subject: [PATCH 1/9] Enable batch processing for GSASII GUI --- .../tabs/gsas2/model.py | 91 ++++++++++++++++++- 1 file changed, 88 insertions(+), 3 deletions(-) diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py index fa7f5c331ec1..08c0dd3d2b9d 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py @@ -12,7 +12,7 @@ import subprocess from dataclasses import dataclass, field from pathlib import Path -from typing import List, Optional, Union, Tuple, TypeAlias +from typing import List, Optional, Union, Tuple, TypeAlias, Dict import matplotlib.pyplot as plt from matplotlib.axes import Axes @@ -194,6 +194,52 @@ def clear_input_components(self) -> None: for attr, default in refinement_defaults.items(): setattr(self, attr, default) + # def run_model( + # self, + # load_parameters: list, + # refinement_parameters: list, + # project_name: str, + # rb_num: Optional[str] = None, + # user_x_limits: Optional[List[List[float]]] = None, + # ) -> Optional[int]: + # self.clear_input_components() + # if not self.initial_validation(project_name, load_parameters): + # return None + # self.set_components_from_inputs(load_parameters, refinement_parameters, project_name, rb_num) + # self.read_phase_files() + # self.generate_reflections_from_space_group() + + # formatted_limits: Optional[List[List[float]]] = None + # # Ensure both elements are lists of floats and pass formatted limits to validate_x_limits + # if isinstance(user_x_limits, list) and len(user_x_limits) == 2: + # formatted_limits = [ + # user_x_limits[0] if isinstance(user_x_limits[0], list) else [user_x_limits[0]], + # user_x_limits[1] if isinstance(user_x_limits[1], list) else [user_x_limits[1]], + # ] + + # self.validate_x_limits(formatted_limits) + # if not self.further_validation(): + # return None + + # runtime = self.call_gsas2() + # if not runtime: + # return None + # runtime_str = runtime[1] # Extract the string part of the runtime tuple + # report_result = self.report_on_outputs(runtime_str) + # if report_result is not None: + # gsas_result_filepath, _ = report_result # Unpack the tuple + # else: + # logger.error("Failed to unpack the result from report_on_outputs.") + # return None + # gsas_result = gsas_result_filepath + # if not gsas_result: + # return None + # self.load_basic_outputs(gsas_result) + + # if self.state.number_of_regions > self.state.number_histograms: + # return self.state.number_of_regions + # return self.state.number_histograms + def run_model( self, load_parameters: list, @@ -201,6 +247,38 @@ def run_model( project_name: str, rb_num: Optional[str] = None, user_x_limits: Optional[List[List[float]]] = None, + ) -> Optional[Dict[str, int]]: + """ + Returns a dictionary mapping data file names to their result counts + """ + data_files = load_parameters[2] # Extract data files list + num_hist = None + + for data_file in data_files: + # Create unique project name for each file + file_basename = os.path.splitext(os.path.basename(data_file))[0] + individual_project_name = f"{project_name}_{file_basename}" + + # Create modified load_parameters for single file + single_file_load_params = [ + load_parameters[0], # instrument_files (reuse same) + load_parameters[1], # phase_filepaths (reuse same) + [data_file], # single data file + ] + + num_hist = self._run_single_refinement( + single_file_load_params, refinement_parameters, individual_project_name, rb_num, user_x_limits + ) + + return num_hist + + def _run_single_refinement( + self, + load_parameters: list, + refinement_parameters: list, + project_name: str, + rb_num: Optional[str] = None, + user_x_limits: Optional[List[List[float]]] = None, ) -> Optional[int]: self.clear_input_components() if not self.initial_validation(project_name, load_parameters): @@ -240,6 +318,9 @@ def run_model( return self.state.number_of_regions return self.state.number_histograms + # C:/MantidInstall/scripts/Engineering/ENGINX/phase_info/ + # C:/Users/joy22959/Engineering_Mantid/User/test/Focus/ + # =============== # Prepare Inputs # =============== @@ -912,9 +993,13 @@ def create_lattice_parameter_table(self, test: bool = False) -> Optional[Union[N # =========== def load_focused_nxs_for_logs(self, filenames: List[str]) -> None: - if len(filenames) == 1 and "all_banks" in filenames[0]: - filenames = [filenames[0].replace("all_banks", "bank_1"), filenames[0].replace("all_banks", "bank_2")] + banks_filenames = [] for filename in filenames: + if "all_banks" in filename: + banks_filenames.extend([filename.replace("all_banks", "bank_1"), filename.replace("all_banks", "bank_2")]) + else: + banks_filenames.append(filename) + for filename in banks_filenames: filename = filename.replace(".gss", ".nxs") ws_name = _generate_workspace_name(filename, self._suffix) if ws_name not in self._data_workspaces.get_loaded_workpace_names(): From 542cb3e6750aed917df8e1d53409a2e56ed05828 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Thu, 25 Sep 2025 09:38:08 +0100 Subject: [PATCH 2/9] Use implementation that run GSASII only once and add more validation --- .../tabs/gsas2/call_G2sc.py | 54 ++++---- .../tabs/gsas2/gsas2_handler.py | 3 + .../tabs/gsas2/model.py | 129 +++++------------- 3 files changed, 62 insertions(+), 124 deletions(-) diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py index 654ddc4b51a5..8e56045d6562 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py @@ -25,37 +25,30 @@ def add_phases(project, phase_files): project.add_phase(os.path.join(phase_file)) -def add_histograms(data_filenames, project, instruments, number_regions): - if number_regions > len(data_filenames): # many regions in one data and one instrument file - if len(data_filenames) != 1: - raise ValueError("There must be one region/bank per focused data file or many regions/banks in one focused data file") - for loop_region_index in range(1, number_regions + 1): +def add_histograms(data_filenames, project, instruments, number_regions, banks_per_file): + # Validation checks + if len(data_filenames) < 1: + raise ValueError("You must provide at least one data file.") + + if len(instruments) != 1: + raise ValueError("You must provide exactly one instrument file.") + + if banks_per_file <= 1: + raise ValueError("Each data file must contain multiple banks.") + + expected_total_regions = len(data_filenames) * banks_per_file + if number_regions != expected_total_regions: + raise ValueError(f"Expected {expected_total_regions} total regions, but found {number_regions}.") + + # Add histograms for each file and each bank within that file + for file_index, data_filename in enumerate(data_filenames): + for bank_index in range(1, banks_per_file + 1): # GSAS-II uses 1-based indexing project.add_powder_histogram( - datafile=os.path.join(data_filenames[0]), - iparams=os.path.join(instruments[0]), + datafile=os.path.join(data_filename), + iparams=os.path.join(instruments[0]), # Use the single instrument file phases=project.phases(), - databank=loop_region_index, # indexing starts at 1 - instbank=loop_region_index, # indexing starts at 1 - ) - else: # one region in each data file - if len(data_filenames) != number_regions: - raise ValueError("There must be one region/bank per focused data file or many regions/banks in one focused data file") - if len(instruments) == len(data_filenames): - for loop_index, loop_data_filename in enumerate(data_filenames): - project.add_powder_histogram( - datafile=os.path.join(loop_data_filename), iparams=os.path.join(instruments[loop_index]), phases=project.phases() - ) - elif 1 == len(instruments) < len(data_filenames): - for loop_index, loop_data_filename in enumerate(data_filenames): - project.add_powder_histogram( - datafile=os.path.join(loop_data_filename), - iparams=os.path.join(instruments[0]), - phases=project.phases(), - instbank=loop_index + 1, # indexing starts at 1 - ) - else: - raise ValueError( - "Calling GSASII from Mantid with multiple instrument files and one focused data file is currently not supported" + databank=bank_index, # Bank within this data file + instbank=bank_index, # Corresponding bank in instrument file ) @@ -232,6 +225,7 @@ def main(): mantid_pawley_reflections = inputs_dict["mantid_pawley_reflections"] d_spacing_min = inputs_dict["d_spacing_min"] number_of_regions = inputs_dict["number_of_regions"] + banks_per_file = inputs_dict["banks_per_file"] gsasii_scriptable_path = inputs_dict["gsasii_scriptable_path"] # Call GSASIIscriptable @@ -241,7 +235,7 @@ def main(): gsas_project = G2sc.G2Project(filename=project_path) add_phases(gsas_project, phase_files) - add_histograms(data_files, gsas_project, instrument_files, number_of_regions) + add_histograms(data_files, gsas_project, instrument_files, number_of_regions, banks_per_file) if refinement_method == "Pawley" and mantid_pawley_reflections: add_pawley_reflections(mantid_pawley_reflections, gsas_project, d_spacing_min) diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/gsas2_handler.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/gsas2_handler.py index 4ea143841094..fab42acac84d 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/gsas2_handler.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/gsas2_handler.py @@ -79,6 +79,7 @@ class GSAS2Config: override_cell_lengths: A list of cell length overrides. d_spacing_min: The minimum d-spacing value. number_of_regions: The number of regions to configure. + banks_per_file: Number of banks per file. """ limits: Optional[List[Union[int, float]]] = field(default_factory=list) @@ -86,6 +87,7 @@ class GSAS2Config: override_cell_lengths: Optional[List[List[float]]] = None d_spacing_min: float = 1.0 number_of_regions: int = 1 + banks_per_file: int = 0 class GSAS2Handler(object): @@ -323,6 +325,7 @@ def to_json(self) -> str: "override_cell_lengths": self.config.override_cell_lengths, "d_spacing_min": self.config.d_spacing_min, "number_of_regions": self.config.number_of_regions, + "banks_per_file": self.config.banks_per_file, "gsasii_scriptable_path": str(self.gsasii_scriptable_path), } return json.dumps(inputs_dict, separators=(",", ":")) diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py index 08c0dd3d2b9d..ad53bd14e21c 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py @@ -12,7 +12,7 @@ import subprocess from dataclasses import dataclass, field from pathlib import Path -from typing import List, Optional, Union, Tuple, TypeAlias, Dict +from typing import List, Optional, Union, Tuple, TypeAlias import matplotlib.pyplot as plt from matplotlib.axes import Axes @@ -125,11 +125,13 @@ class GSAS2RuntimeState: Attributes: number_of_regions: Number of regions in the model. number_histograms: Number of histograms in the model. + banks_per_file: Number of banks per file. d_spacing_min: The minimum d-spacing value. """ number_of_regions: int = 0 number_histograms: int = 0 + banks_per_file: int = 0 d_spacing_min: float = 1.0 @@ -194,52 +196,6 @@ def clear_input_components(self) -> None: for attr, default in refinement_defaults.items(): setattr(self, attr, default) - # def run_model( - # self, - # load_parameters: list, - # refinement_parameters: list, - # project_name: str, - # rb_num: Optional[str] = None, - # user_x_limits: Optional[List[List[float]]] = None, - # ) -> Optional[int]: - # self.clear_input_components() - # if not self.initial_validation(project_name, load_parameters): - # return None - # self.set_components_from_inputs(load_parameters, refinement_parameters, project_name, rb_num) - # self.read_phase_files() - # self.generate_reflections_from_space_group() - - # formatted_limits: Optional[List[List[float]]] = None - # # Ensure both elements are lists of floats and pass formatted limits to validate_x_limits - # if isinstance(user_x_limits, list) and len(user_x_limits) == 2: - # formatted_limits = [ - # user_x_limits[0] if isinstance(user_x_limits[0], list) else [user_x_limits[0]], - # user_x_limits[1] if isinstance(user_x_limits[1], list) else [user_x_limits[1]], - # ] - - # self.validate_x_limits(formatted_limits) - # if not self.further_validation(): - # return None - - # runtime = self.call_gsas2() - # if not runtime: - # return None - # runtime_str = runtime[1] # Extract the string part of the runtime tuple - # report_result = self.report_on_outputs(runtime_str) - # if report_result is not None: - # gsas_result_filepath, _ = report_result # Unpack the tuple - # else: - # logger.error("Failed to unpack the result from report_on_outputs.") - # return None - # gsas_result = gsas_result_filepath - # if not gsas_result: - # return None - # self.load_basic_outputs(gsas_result) - - # if self.state.number_of_regions > self.state.number_histograms: - # return self.state.number_of_regions - # return self.state.number_histograms - def run_model( self, load_parameters: list, @@ -247,38 +203,6 @@ def run_model( project_name: str, rb_num: Optional[str] = None, user_x_limits: Optional[List[List[float]]] = None, - ) -> Optional[Dict[str, int]]: - """ - Returns a dictionary mapping data file names to their result counts - """ - data_files = load_parameters[2] # Extract data files list - num_hist = None - - for data_file in data_files: - # Create unique project name for each file - file_basename = os.path.splitext(os.path.basename(data_file))[0] - individual_project_name = f"{project_name}_{file_basename}" - - # Create modified load_parameters for single file - single_file_load_params = [ - load_parameters[0], # instrument_files (reuse same) - load_parameters[1], # phase_filepaths (reuse same) - [data_file], # single data file - ] - - num_hist = self._run_single_refinement( - single_file_load_params, refinement_parameters, individual_project_name, rb_num, user_x_limits - ) - - return num_hist - - def _run_single_refinement( - self, - load_parameters: list, - refinement_parameters: list, - project_name: str, - rb_num: Optional[str] = None, - user_x_limits: Optional[List[List[float]]] = None, ) -> Optional[int]: self.clear_input_components() if not self.initial_validation(project_name, load_parameters): @@ -295,7 +219,8 @@ def _run_single_refinement( user_x_limits[1] if isinstance(user_x_limits[1], list) else [user_x_limits[1]], ] - self.validate_x_limits(formatted_limits) + if not self.validate_x_limits(formatted_limits): + return None if not self.further_validation(): return None @@ -314,12 +239,7 @@ def _run_single_refinement( return None self.load_basic_outputs(gsas_result) - if self.state.number_of_regions > self.state.number_histograms: - return self.state.number_of_regions - return self.state.number_histograms - - # C:/MantidInstall/scripts/Engineering/ENGINX/phase_info/ - # C:/Users/joy22959/Engineering_Mantid/User/test/Focus/ + return self.state.number_of_regions # =============== # Prepare Inputs @@ -420,6 +340,7 @@ def _create_gsas2_config(self) -> GSAS2Config: override_cell_lengths=self.get_override_lattice_parameters(), d_spacing_min=self.dSpacing_min, number_of_regions=self.state.number_of_regions, + banks_per_file=self.state.banks_per_file, ) def _initialize_gsas2_handler( @@ -650,31 +571,51 @@ def generate_reflections_from_space_group(self) -> None: # ========= def understand_data_structure(self) -> None: + if len(self.file_paths.instrument_files) != 1: + logger.error("* The number of instrument files must be one.") + return False + + if len(self.file_paths.instrument_files) != 1: + logger.error("* You must provide exactly one instrument file.") + return False + self.x_limits.data_x_min = [] self.x_limits.data_x_max = [] number_of_regions = 0 + banks_per_file = [] # Track banks per file for validation + for input_file in self.file_paths.data_files: loop_focused_workspace = LoadGSS(Filename=input_file, OutputWorkspace="GSASII_input_data", EnableLogging=False) - for workspace_index in range(loop_focused_workspace.getNumberHistograms()): + file_bank_count = loop_focused_workspace.getNumberHistograms() + banks_per_file.append(file_bank_count) + + if file_bank_count <= 1: + logger.error("* Each data file must contain multiple banks.") + return False + + for workspace_index in range(file_bank_count): self.x_limits.data_x_min.append(loop_focused_workspace.readX(workspace_index)[0]) self.x_limits.data_x_max.append(loop_focused_workspace.readX(workspace_index)[-1]) number_of_regions += 1 DeleteWorkspace(loop_focused_workspace) + + expected_total_regions = len(self.file_paths.data_files) * banks_per_file[0] + if number_of_regions != expected_total_regions: + logger.error(f"* Expected {expected_total_regions} total regions, but found {number_of_regions}.") + return False + self.state.number_of_regions = number_of_regions + self.state.banks_per_file = banks_per_file[0] if banks_per_file else 0 + return True def validate_x_limits(self, users_limits: Optional[List[List[float]]]) -> bool: - self.understand_data_structure() + if not self.understand_data_structure(): + return False if users_limits: if len(users_limits[0]) != self.state.number_of_regions: users_limits[0] *= self.state.number_of_regions users_limits[1] *= self.state.number_of_regions self.state.number_histograms = len(self.file_paths.data_files) - if len(self.file_paths.instrument_files) != 1 and len(self.file_paths.instrument_files) != self.state.number_histograms: - logger.error( - f"The number of instrument files ({len(self.file_paths.instrument_files)}) must be 1 " - f"or equal to the number of input histograms {self.state.number_histograms}" - ) - return False if users_limits: self.x_limits.x_min = [float(k) for k in users_limits[0]] self.x_limits.x_max = [float(k) for k in users_limits[1]] From e2169681778b50f3ba523bf5bc7acf2e97ac510b Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Thu, 25 Sep 2025 09:38:57 +0100 Subject: [PATCH 3/9] Update unit tests to cover the new cases --- .../tabs/gsas2/test/test_call_G2sc.py | 79 +++++++++---------- .../tabs/gsas2/test/test_gsas2_model.py | 11 +++ 2 files changed, 47 insertions(+), 43 deletions(-) diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_call_G2sc.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_call_G2sc.py index 7207ba0d9b39..37b609315d0b 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_call_G2sc.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_call_G2sc.py @@ -53,70 +53,63 @@ def test_add_phases(self): add_phases(self.project, phase_files) self.project.add_phase.assert_has_calls([mock.call("file_1"), mock.call("file_2")]) - def test_add_histograms_with_single_datafile(self): + def test_add_histograms_with_single_datafile_multiple_banks(self): data_filenames = ["data_file_1"] instruments = ["instr_1"] number_of_regions = 3 - add_histograms(data_filenames, self.project, instruments, number_of_regions) + banks_per_file = 3 + add_histograms(data_filenames, self.project, instruments, number_of_regions, banks_per_file) self.project.add_powder_histogram.assert_called() + self.assertEqual(self.project.add_powder_histogram.call_count, 3) - def test_add_histograms_same_number_of_instruments_and_datafiles(self): - self.project.phases.return_value = ["phase_1"] - data_filenames = ["data_file_1", "data_file_2"] - instruments = ["instr_1", "instr_2"] - number_of_regions = 2 - add_histograms(data_filenames, self.project, instruments, number_of_regions) - expected_calls = [ - mock.call(datafile="data_file_1", iparams="instr_1", phases=["phase_1"]), - mock.call(datafile="data_file_2", iparams="instr_2", phases=["phase_1"]), - ] - self.project.add_powder_histogram.assert_has_calls(expected_calls) - - def test_add_histograms_more_datafiles_than_instruments(self): + def test_add_histograms_more_datafiles_one_instrument(self): self.project.phases.return_value = ["phase_1"] data_filenames = ["data_file_1", "data_file_2"] instruments = ["instr_1"] - number_of_regions = 2 - add_histograms(data_filenames, self.project, instruments, number_of_regions) - expected_calls = [ - mock.call(datafile="data_file_1", iparams="instr_1", phases=["phase_1"], instbank=1), - mock.call(datafile="data_file_2", iparams="instr_1", phases=["phase_1"], instbank=2), - ] - self.project.add_powder_histogram.assert_has_calls(expected_calls) + number_of_regions = 6 # 2 files * 3 banks each + banks_per_file = 3 + add_histograms(data_filenames, self.project, instruments, number_of_regions, banks_per_file) - def test_add_histograms_multiple_regions_for_single_datafile(self): - self.project.phases.return_value = ["phase_1"] - data_filenames = ["data_file_1"] - instruments = ["instr_1", "instr_2"] - number_of_regions = 3 - add_histograms(data_filenames, self.project, instruments, number_of_regions) expected_calls = [ + # File 1, banks 1-3 mock.call(datafile="data_file_1", iparams="instr_1", phases=["phase_1"], databank=1, instbank=1), mock.call(datafile="data_file_1", iparams="instr_1", phases=["phase_1"], databank=2, instbank=2), mock.call(datafile="data_file_1", iparams="instr_1", phases=["phase_1"], databank=3, instbank=3), + # File 2, banks 1-3 + mock.call(datafile="data_file_2", iparams="instr_1", phases=["phase_1"], databank=1, instbank=1), + mock.call(datafile="data_file_2", iparams="instr_1", phases=["phase_1"], databank=2, instbank=2), + mock.call(datafile="data_file_2", iparams="instr_1", phases=["phase_1"], databank=3, instbank=3), ] self.project.add_powder_histogram.assert_has_calls(expected_calls) - def test_add_histograms_throws_for_more_datafiles_than_number_of_regions(self): - data_filenames = ["data_file_1", "data_file_2"] - instruments = ["instr_1", "instr_2"] - number_of_regions = 3 - with self.assertRaises(ValueError): - add_histograms(data_filenames, self.project, instruments, number_of_regions) + def test_add_histograms_throws_for_single_bank_per_file(self): + """Test that single bank per file is rejected""" + data_filenames = ["data_file_1"] + instruments = ["instr_1"] + number_of_regions = 1 + banks_per_file = 1 + with self.assertRaises(ValueError) as context: + add_histograms(data_filenames, self.project, instruments, number_of_regions, banks_per_file) + self.assertIn("Each data file must contain multiple banks", str(context.exception)) - def test_add_histograms_throws_for_less_datafiles_than_number_of_regions(self): - data_filenames = ["data_file_1", "data_file_2"] + def test_add_histograms_throws_for_multiple_instruments_for_single_datafile(self): + self.project.phases.return_value = ["phase_1"] + data_filenames = ["data_file_1"] instruments = ["instr_1", "instr_2"] - number_of_regions = 1 + number_of_regions = 3 + banks_per_file = 3 with self.assertRaises(ValueError): - add_histograms(data_filenames, self.project, instruments, number_of_regions) + add_histograms(data_filenames, self.project, instruments, number_of_regions, banks_per_file) - def test_add_histograms_throws_for_less_datafiles_than_instruments(self): + def test_add_histograms_throws_for_inconsistent_region_count(self): + """Test that inconsistent total region count is rejected""" data_filenames = ["data_file_1", "data_file_2"] - instruments = ["instr_1", "instr_2", "instr_3"] - number_of_regions = 2 - with self.assertRaises(ValueError): - add_histograms(data_filenames, self.project, instruments, number_of_regions) + instruments = ["instr_1"] + number_of_regions = 5 # Should be 6 for 2 files * 3 banks + banks_per_file = 3 + with self.assertRaises(ValueError) as context: + add_histograms(data_filenames, self.project, instruments, number_of_regions, banks_per_file) + self.assertIn("Expected 6 total regions, but found 5", str(context.exception)) def test_add_pawley_reflections(self): phase_1 = mock.Mock() diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_gsas2_model.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_gsas2_model.py index 38e074f936f3..3cadd494c505 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_gsas2_model.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_gsas2_model.py @@ -130,6 +130,17 @@ def test_run_model_with_correctly_formatted_user_x_limits(self, mock_validate_x_ self.model.run_model(load_parameters, refinement_parameters, "test_project", user_x_limits=formatted_user_x_limits) mock_validate_x_limits.assert_called_once_with(formatted_user_x_limits) + @patch("mantidqtinterfaces.Engineering.gui.engineering_diffraction.tabs.gsas2.model.LoadCIF") + @patch("mantidqtinterfaces.Engineering.gui.engineering_diffraction.tabs.gsas2.model.GSAS2Model.validate_x_limits") + def test_run_model_with_multiple_datafiles(self, mock_validate_x_limits, mock_load_cif): + self._setup_crystal_structure_and_mocks(mock_load_cif) + + load_parameters = ["mock_instrument", ["mock_phase.cif"], ["mock_data.gss", "mock_data2.gss", "mock_data3.gss"]] + refinement_parameters = ["Pawley", None, False, False, False] + user_x_limits = [[10000.0, 20000.0], [30000.0, 40000.0]] + + self.model.run_model(load_parameters, refinement_parameters, "test_project", user_x_limits=user_x_limits) + @patch("mantidqtinterfaces.Engineering.gui.engineering_diffraction.tabs.gsas2.model.LoadCIF") @patch("mantidqtinterfaces.Engineering.gui.engineering_diffraction.tabs.gsas2.model.GSAS2Model.validate_x_limits") def test_run_model_with_unformatted_user_x_limits(self, mock_validate_x_limits, mock_load_cif): From 0648d7e54c6297c05eaca761d155aa0e76fe8f43 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Thu, 25 Sep 2025 09:42:37 +0100 Subject: [PATCH 4/9] Update test guide documentation --- .../EngineeringDiffractionTestGuide.rst | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/dev-docs/source/Testing/EngineeringDiffraction/EngineeringDiffractionTestGuide.rst b/dev-docs/source/Testing/EngineeringDiffraction/EngineeringDiffractionTestGuide.rst index a34fe20ec4d0..b510cda22203 100644 --- a/dev-docs/source/Testing/EngineeringDiffraction/EngineeringDiffractionTestGuide.rst +++ b/dev-docs/source/Testing/EngineeringDiffraction/EngineeringDiffractionTestGuide.rst @@ -315,3 +315,29 @@ Please test this on IDAaaS: an ENGINX instance should have MantidWorkbenchNightl 11. Set the ``Override Unit Cell Length`` to ``3.65`` and click ``Refine in GSAS II``, the fit should be better. 12. Tick all the checkboxes: ``Microstrain``, ``Sigma-1`` and ``Gamma (Y)``. An asterisk should appear with an advice tooltip. + +Test 12 +^^^^^^^ +This test covers the multiple data files functionality with multiple banks per file in the ``GSASII`` tab. + +Note this test will only work if ``GSASII`` is also installed. +Please test this on IDAaaS: an ENGINX instance should have MantidWorkbenchNightly and ``GSASII`` installed in the expected location. + +1. Close and re-open the Engineering Diffraction interface. + +2. Go to the ``Calibration`` tab, select ``Create New Calibration`` and un-tick the ``Set Calibration Region of Interest`` option. + +3. For the ``Calibration Sample`` # enter ``305738`` and click the ``Calibrate`` button. + +4. On the ``Focus`` tab, enter Sample Run # ``305793-305795`` and Vanadium # ``307521`` and click the Focus button. This will generate multiple focused data files. +Change to the GSASII tab. Clear any pre-filled paths. + +5. For the ``Instrument Group`` filepath, browse and select the single .prm file output by the calibration (should be ENGINX_305738_all_banks.prm). + +6. For the ``Focused Data`` filepath, browse and select multiple .gss files that each contain multiple banks. Ensure all selected files have the same number of banks (e.g., select the all_banks files: ENGINX_305738_305793_all_banks_dSpacing.gss, ENGINX_305738_305794_all_banks_dSpacing.gss, ENGINX_305738_305795_all_banks_dSpacing.gss). + +7. For the ``Phase`` filepath, browse to MANTID_INSTALL_DIRECTORY/scripts/Engineering/ENGINX/phase_info/FE_GAMMA.cif. For the ``Project Name`` at the top, enter a string of your choice. + +8. Click Refine in ``GSAS II``. After a few seconds, the output fit should be plotted. In the top right of the plot widget, verify that the refined spectrum combobox shows entries for the banks of the last refined data file. + +9. Test Error Cases: Try selecting multiple instrument .prm files (should show error message about requiring exactly one instrument file). Try selecting .gss files with different numbers of banks (should show error about inconsistent bank counts). Try selecting single-bank .gss files (should show error about requiring multiple banks per file). From cadf64cd2d7bbce25b692383fb6196294a6a6b44 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Thu, 25 Sep 2025 09:43:32 +0100 Subject: [PATCH 5/9] Update user guide documentation --- .../source/interfaces/diffraction/Engineering Diffraction.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/interfaces/diffraction/Engineering Diffraction.rst b/docs/source/interfaces/diffraction/Engineering Diffraction.rst index b4f6adb7e20a..a159f620d896 100644 --- a/docs/source/interfaces/diffraction/Engineering Diffraction.rst +++ b/docs/source/interfaces/diffraction/Engineering Diffraction.rst @@ -230,14 +230,14 @@ Project Name Name of the GSAS project file. Instrument Group - Path to .prm file produced by the Calibration tab + Path to .prm file produced by the Calibration tab (only one instrument file is supported and will be applied to each data file) Phase Path to the .cif file defining the initial crystal structure (more than one path can be supplied, the lattice parameters will be overridden for the first phase only). Focused Data - Path to focused .gss files (note it should have the same number of spectra as in .prm file) + Path to focused .gss files (note it should have the same number of spectra as in .prm file and contains multiple banks) Refinement Method Only Pawley refinement currently supported From e297df4afc9ea34a2372ed78ea75ba71ba4dc968 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Thu, 25 Sep 2025 09:52:50 +0100 Subject: [PATCH 6/9] Add release note --- .../v6.14.0/Diffraction/Engineering/New_features/39963.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 docs/source/release/v6.14.0/Diffraction/Engineering/New_features/39963.rst diff --git a/docs/source/release/v6.14.0/Diffraction/Engineering/New_features/39963.rst b/docs/source/release/v6.14.0/Diffraction/Engineering/New_features/39963.rst new file mode 100644 index 000000000000..0cec6830c40a --- /dev/null +++ b/docs/source/release/v6.14.0/Diffraction/Engineering/New_features/39963.rst @@ -0,0 +1,2 @@ +- Support batch refinement of multi-run datasets using a single instrument parameter file in :ref:`GSASII tab ` tab of :ref:`Engineering Diffraction interface` GUI. +- Removed support for mixed single-region and multi-instrument file configurations in :ref:`GSASII tab ` tab of :ref:`Engineering Diffraction interface` GUI. From ffc73207e3d98d72810fa47fdeb7ca2a48a2a086 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Thu, 25 Sep 2025 15:01:57 +0100 Subject: [PATCH 7/9] Revert back to multiple gsas calls --- .../tabs/gsas2/call_G2sc.py | 42 ++++++++--------- .../tabs/gsas2/gsas2_handler.py | 3 -- .../tabs/gsas2/model.py | 45 +++++++++++++++---- .../tabs/gsas2/test/test_call_G2sc.py | 42 ++++------------- .../tabs/gsas2/test/test_gsas2_model.py | 31 +++++++------ 5 files changed, 80 insertions(+), 83 deletions(-) diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py index 8e56045d6562..a4276f37517e 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py @@ -25,31 +25,26 @@ def add_phases(project, phase_files): project.add_phase(os.path.join(phase_file)) -def add_histograms(data_filenames, project, instruments, number_regions, banks_per_file): +def add_histograms(data_filenames, project, instruments, number_regions): # Validation checks - if len(data_filenames) < 1: - raise ValueError("You must provide at least one data file.") + if len(data_filenames) != 1: + raise ValueError("You must provide only one data file.") if len(instruments) != 1: - raise ValueError("You must provide exactly one instrument file.") - - if banks_per_file <= 1: - raise ValueError("Each data file must contain multiple banks.") - - expected_total_regions = len(data_filenames) * banks_per_file - if number_regions != expected_total_regions: - raise ValueError(f"Expected {expected_total_regions} total regions, but found {number_regions}.") - - # Add histograms for each file and each bank within that file - for file_index, data_filename in enumerate(data_filenames): - for bank_index in range(1, banks_per_file + 1): # GSAS-II uses 1-based indexing - project.add_powder_histogram( - datafile=os.path.join(data_filename), - iparams=os.path.join(instruments[0]), # Use the single instrument file - phases=project.phases(), - databank=bank_index, # Bank within this data file - instbank=bank_index, # Corresponding bank in instrument file - ) + raise ValueError("You must provide only one instrument file.") + + if number_regions <= 1: + raise ValueError("The data file must contain multiple banks.") + + # Add histograms for each bank within that file + for bank_index in range(1, number_regions + 1): # GSAS-II uses 1-based indexing + project.add_powder_histogram( + datafile=os.path.join(data_filenames[0]), + iparams=os.path.join(instruments[0]), # Use the single instrument file + phases=project.phases(), + databank=bank_index, # Bank within this data file + instbank=bank_index, # Corresponding bank in instrument file + ) def add_pawley_reflections(pawley_reflections, project, d_min): @@ -225,7 +220,6 @@ def main(): mantid_pawley_reflections = inputs_dict["mantid_pawley_reflections"] d_spacing_min = inputs_dict["d_spacing_min"] number_of_regions = inputs_dict["number_of_regions"] - banks_per_file = inputs_dict["banks_per_file"] gsasii_scriptable_path = inputs_dict["gsasii_scriptable_path"] # Call GSASIIscriptable @@ -235,7 +229,7 @@ def main(): gsas_project = G2sc.G2Project(filename=project_path) add_phases(gsas_project, phase_files) - add_histograms(data_files, gsas_project, instrument_files, number_of_regions, banks_per_file) + add_histograms(data_files, gsas_project, instrument_files, number_of_regions) if refinement_method == "Pawley" and mantid_pawley_reflections: add_pawley_reflections(mantid_pawley_reflections, gsas_project, d_spacing_min) diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/gsas2_handler.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/gsas2_handler.py index fab42acac84d..4ea143841094 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/gsas2_handler.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/gsas2_handler.py @@ -79,7 +79,6 @@ class GSAS2Config: override_cell_lengths: A list of cell length overrides. d_spacing_min: The minimum d-spacing value. number_of_regions: The number of regions to configure. - banks_per_file: Number of banks per file. """ limits: Optional[List[Union[int, float]]] = field(default_factory=list) @@ -87,7 +86,6 @@ class GSAS2Config: override_cell_lengths: Optional[List[List[float]]] = None d_spacing_min: float = 1.0 number_of_regions: int = 1 - banks_per_file: int = 0 class GSAS2Handler(object): @@ -325,7 +323,6 @@ def to_json(self) -> str: "override_cell_lengths": self.config.override_cell_lengths, "d_spacing_min": self.config.d_spacing_min, "number_of_regions": self.config.number_of_regions, - "banks_per_file": self.config.banks_per_file, "gsasii_scriptable_path": str(self.gsasii_scriptable_path), } return json.dumps(inputs_dict, separators=(",", ":")) diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py index ad53bd14e21c..23f8a941a583 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py @@ -12,7 +12,7 @@ import subprocess from dataclasses import dataclass, field from pathlib import Path -from typing import List, Optional, Union, Tuple, TypeAlias +from typing import Dict, List, Optional, Union, Tuple, TypeAlias import matplotlib.pyplot as plt from matplotlib.axes import Axes @@ -125,13 +125,11 @@ class GSAS2RuntimeState: Attributes: number_of_regions: Number of regions in the model. number_histograms: Number of histograms in the model. - banks_per_file: Number of banks per file. d_spacing_min: The minimum d-spacing value. """ number_of_regions: int = 0 number_histograms: int = 0 - banks_per_file: int = 0 d_spacing_min: float = 1.0 @@ -203,6 +201,41 @@ def run_model( project_name: str, rb_num: Optional[str] = None, user_x_limits: Optional[List[List[float]]] = None, + ) -> Optional[Dict[str, int]]: + """ + Returns a dictionary mapping data file names to their result counts + """ + data_files = load_parameters[2] # Extract data files list + num_hist = None + + for data_file in data_files: + # Create unique project name for each file + file_basename = os.path.splitext(os.path.basename(data_file))[0] + individual_project_name = f"{project_name}_{file_basename}" + + # Create modified load_parameters for single file + single_file_load_params = [ + load_parameters[0], # instrument_files (reuse same) + load_parameters[1], # phase_filepaths (reuse same) + [data_file], # single data file + ] + + num_hist = self._run_single_refinement( + single_file_load_params, refinement_parameters, individual_project_name, rb_num, user_x_limits + ) + + if not num_hist: + return + + return num_hist + + def _run_single_refinement( + self, + load_parameters: list, + refinement_parameters: list, + project_name: str, + rb_num: Optional[str] = None, + user_x_limits: Optional[List[List[float]]] = None, ) -> Optional[int]: self.clear_input_components() if not self.initial_validation(project_name, load_parameters): @@ -340,7 +373,6 @@ def _create_gsas2_config(self) -> GSAS2Config: override_cell_lengths=self.get_override_lattice_parameters(), d_spacing_min=self.dSpacing_min, number_of_regions=self.state.number_of_regions, - banks_per_file=self.state.banks_per_file, ) def _initialize_gsas2_handler( @@ -571,10 +603,6 @@ def generate_reflections_from_space_group(self) -> None: # ========= def understand_data_structure(self) -> None: - if len(self.file_paths.instrument_files) != 1: - logger.error("* The number of instrument files must be one.") - return False - if len(self.file_paths.instrument_files) != 1: logger.error("* You must provide exactly one instrument file.") return False @@ -605,7 +633,6 @@ def understand_data_structure(self) -> None: return False self.state.number_of_regions = number_of_regions - self.state.banks_per_file = banks_per_file[0] if banks_per_file else 0 return True def validate_x_limits(self, users_limits: Optional[List[List[float]]]) -> bool: diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_call_G2sc.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_call_G2sc.py index 37b609315d0b..aa67627f5f37 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_call_G2sc.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_call_G2sc.py @@ -57,59 +57,33 @@ def test_add_histograms_with_single_datafile_multiple_banks(self): data_filenames = ["data_file_1"] instruments = ["instr_1"] number_of_regions = 3 - banks_per_file = 3 - add_histograms(data_filenames, self.project, instruments, number_of_regions, banks_per_file) + add_histograms(data_filenames, self.project, instruments, number_of_regions) self.project.add_powder_histogram.assert_called() self.assertEqual(self.project.add_powder_histogram.call_count, 3) - def test_add_histograms_more_datafiles_one_instrument(self): + def test_add_histograms_throws_for_more_datafiles_one_instrument(self): self.project.phases.return_value = ["phase_1"] data_filenames = ["data_file_1", "data_file_2"] instruments = ["instr_1"] - number_of_regions = 6 # 2 files * 3 banks each - banks_per_file = 3 - add_histograms(data_filenames, self.project, instruments, number_of_regions, banks_per_file) - - expected_calls = [ - # File 1, banks 1-3 - mock.call(datafile="data_file_1", iparams="instr_1", phases=["phase_1"], databank=1, instbank=1), - mock.call(datafile="data_file_1", iparams="instr_1", phases=["phase_1"], databank=2, instbank=2), - mock.call(datafile="data_file_1", iparams="instr_1", phases=["phase_1"], databank=3, instbank=3), - # File 2, banks 1-3 - mock.call(datafile="data_file_2", iparams="instr_1", phases=["phase_1"], databank=1, instbank=1), - mock.call(datafile="data_file_2", iparams="instr_1", phases=["phase_1"], databank=2, instbank=2), - mock.call(datafile="data_file_2", iparams="instr_1", phases=["phase_1"], databank=3, instbank=3), - ] - self.project.add_powder_histogram.assert_has_calls(expected_calls) + number_of_regions = 6 + with self.assertRaises(ValueError): + add_histograms(data_filenames, self.project, instruments, number_of_regions) def test_add_histograms_throws_for_single_bank_per_file(self): """Test that single bank per file is rejected""" data_filenames = ["data_file_1"] instruments = ["instr_1"] number_of_regions = 1 - banks_per_file = 1 - with self.assertRaises(ValueError) as context: - add_histograms(data_filenames, self.project, instruments, number_of_regions, banks_per_file) - self.assertIn("Each data file must contain multiple banks", str(context.exception)) + with self.assertRaises(ValueError): + add_histograms(data_filenames, self.project, instruments, number_of_regions) def test_add_histograms_throws_for_multiple_instruments_for_single_datafile(self): self.project.phases.return_value = ["phase_1"] data_filenames = ["data_file_1"] instruments = ["instr_1", "instr_2"] number_of_regions = 3 - banks_per_file = 3 with self.assertRaises(ValueError): - add_histograms(data_filenames, self.project, instruments, number_of_regions, banks_per_file) - - def test_add_histograms_throws_for_inconsistent_region_count(self): - """Test that inconsistent total region count is rejected""" - data_filenames = ["data_file_1", "data_file_2"] - instruments = ["instr_1"] - number_of_regions = 5 # Should be 6 for 2 files * 3 banks - banks_per_file = 3 - with self.assertRaises(ValueError) as context: - add_histograms(data_filenames, self.project, instruments, number_of_regions, banks_per_file) - self.assertIn("Expected 6 total regions, but found 5", str(context.exception)) + add_histograms(data_filenames, self.project, instruments, number_of_regions) def test_add_pawley_reflections(self): phase_1 = mock.Mock() diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_gsas2_model.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_gsas2_model.py index 3cadd494c505..a3ad2bd28066 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_gsas2_model.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/test/test_gsas2_model.py @@ -112,7 +112,7 @@ def test_initial_validation(self): @patch("mantidqtinterfaces.Engineering.gui.engineering_diffraction.tabs.gsas2.model.GSAS2Model.validate_x_limits") def test_run_model_with_user_x_limits_none(self, mock_validate_x_limits, mock_load_cif): self._setup_crystal_structure_and_mocks(mock_load_cif) - load_parameters = ["mock_instrument", ["mock_phase.cif"], ["mock_data.gss"]] + load_parameters = [["mock_instrument"], ["mock_phase.cif"], ["mock_data.gss"]] refinement_parameters = ["Pawley", None, False, False, False] self.model.run_model(load_parameters, refinement_parameters, "test_project", user_x_limits=None) @@ -123,7 +123,7 @@ def test_run_model_with_user_x_limits_none(self, mock_validate_x_limits, mock_lo def test_run_model_with_correctly_formatted_user_x_limits(self, mock_validate_x_limits, mock_load_cif): self._setup_crystal_structure_and_mocks(mock_load_cif) - load_parameters = ["mock_instrument", ["mock_phase.cif"], ["mock_data.gss"]] + load_parameters = [["mock_instrument"], ["mock_phase.cif"], ["mock_data.gss"]] refinement_parameters = ["Pawley", None, False, False, False] formatted_user_x_limits = [[10000.0, 20000.0], [30000.0, 40000.0]] @@ -135,7 +135,7 @@ def test_run_model_with_correctly_formatted_user_x_limits(self, mock_validate_x_ def test_run_model_with_multiple_datafiles(self, mock_validate_x_limits, mock_load_cif): self._setup_crystal_structure_and_mocks(mock_load_cif) - load_parameters = ["mock_instrument", ["mock_phase.cif"], ["mock_data.gss", "mock_data2.gss", "mock_data3.gss"]] + load_parameters = [[["mock_instrument"]], ["mock_phase.cif"], ["mock_data.gss", "mock_data2.gss", "mock_data3.gss"]] refinement_parameters = ["Pawley", None, False, False, False] user_x_limits = [[10000.0, 20000.0], [30000.0, 40000.0]] @@ -146,7 +146,7 @@ def test_run_model_with_multiple_datafiles(self, mock_validate_x_limits, mock_lo def test_run_model_with_unformatted_user_x_limits(self, mock_validate_x_limits, mock_load_cif): self._setup_crystal_structure_and_mocks(mock_load_cif) - load_parameters = ["mock_instrument", ["mock_phase.cif"], ["mock_data.gss"]] + load_parameters = [["mock_instrument"], ["mock_phase.cif"], ["mock_data.gss"]] refinement_parameters = ["Pawley", None, False, False, False] unformatted_user_x_limits = [17522.26, 42558.08] @@ -233,14 +233,19 @@ def test_read_phase_files_with_override_lattice(self): [3.5, 3.5, 3.5, 90.0, 90.0, 90.0], ) - def test_understand_data_structure(self): + @patch("mantidqtinterfaces.Engineering.gui.engineering_diffraction.tabs.gsas2.model.LoadGSS") + def test_understand_data_structure_mutliple_inst_fails(self, mock_gss): + self.model.file_paths.data_files = ["data.gss"] + self.model.file_paths.instrument_files = ["first", "second"] + self.assertEqual(self.model.understand_data_structure(), False) + + @patch("mantidqtinterfaces.Engineering.gui.engineering_diffraction.tabs.gsas2.model.LoadGSS") + def test_understand_data_structure_one_banks_fails(self, mock_gss): # note this gss-ExtendedHeader.gsa test file is not widely relevant for this interface gsa_file_path = FileFinder.getFullPath("gss-ExtendedHeader.gsa") self.model.file_paths.data_files = [gsa_file_path] self.model.understand_data_structure() - self.assertEqual(self.model.state.number_of_regions, 1) - self.assertEqual(self.model.x_limits.data_x_min, [41.7276698840232]) - self.assertEqual(self.model.x_limits.data_x_max, [41.82791649202319]) + self.assertEqual(self.model.understand_data_structure(), False) @patch(model_path + ".GSAS2Model.find_in_file") def test_crystal_params_from_instrument_split_on_spaces(self, mock_find_in_file): @@ -255,7 +260,7 @@ def test_crystal_params_from_instrument_file_mismatch(self, mock_find_in_file): @patch(model_path + ".GSAS2Model.find_in_file") def test_crystal_params_from_instrument_split_on_tabs(self, mock_find_in_file): mock_find_in_file.return_value = "18000\t3.0\t14.0" - self.assertEqual(self.model.get_crystal_params_from_instrument("mock_instrument"), [18017.0]) + self.assertEqual(self.model.get_crystal_params_from_instrument(["mock_instrument"]), [18017.0]) @patch(model_path + ".GSAS2Model.find_in_file") def test_determine_tof_min(self, mock_find_in_file): @@ -263,21 +268,21 @@ def test_determine_tof_min(self, mock_find_in_file): mock_find_in_file.return_value = "18000\t3.0\t14.0" self.model.file_paths.instrument_files = ["first", "second"] self.model.state.number_of_regions = 2 - self.assertEqual(self.model.get_crystal_params_from_instrument("mock_instrument"), [18017.0, 18017.0]) + self.assertEqual(self.model.get_crystal_params_from_instrument(["mock_instrument"]), [18017.0, 18017.0]) # 1 instrument file self.model.file_paths.instrument_files = ["first file containing 2 banks"] self.model.state.number_of_regions = 2 - self.assertEqual(self.model.get_crystal_params_from_instrument("mock_instrument"), [18017.0, 18017.0]) + self.assertEqual(self.model.get_crystal_params_from_instrument(["mock_instrument"]), [18017.0, 18017.0]) @patch(model_path + ".GSAS2Model.understand_data_structure") def test_validate_x_limits(self, mock_understand_data): self.model.state.number_of_regions = 1 - self.model.file_paths.instrument_files = ["inst1", "inst2"] + self.model.file_paths.instrument_files = ["inst1"] self.model.file_paths.data_files = ["data1", "data2", "data3"] two_inst_three_histograms = self.model.validate_x_limits([[18000], [50000]]) - self.assertEqual(two_inst_three_histograms, False) + self.assertEqual(two_inst_three_histograms, True) self.model.file_paths.instrument_files = ["inst1"] one_inst_three_histograms = self.model.validate_x_limits([[18000], [50000]]) From 091218eecf9008ffe23f7af2b60e79f9c409c2a6 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Thu, 25 Sep 2025 15:26:57 +0100 Subject: [PATCH 8/9] Handle mismatched histogram inst file error --- .../gui/engineering_diffraction/tabs/gsas2/model.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py index 23f8a941a583..4754b7ba3311 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py @@ -460,7 +460,14 @@ def call_subprocess(self, command_string_list: List[str], gsas_binary_paths: Lis shell_output = shell_process.communicate(timeout=self.config.timeout) if shell_process.returncode != 0: - logger.error(f"GSAS-II call failed with error: {shell_output[-1]}") + if "not enough values to unpack (expected 2, got 0)" in shell_output[-1]: + # The error message happens when it fails to read inst param file with the same number of histograms + logger.error( + "* Incorrect instrument file. please use the instrument filewith the same number of histograms as the focused data." + ) + else: + logger.error(f"GSAS-II call failed with error: {shell_output[-1]}") + return None return shell_output except subprocess.TimeoutExpired: From 0734a52f6dacb93fb485b52d3a411a18436108e9 Mon Sep 17 00:00:00 2001 From: Mohammed Almakki Date: Thu, 25 Sep 2025 16:34:37 +0100 Subject: [PATCH 9/9] Verify prm file has the same no of banks as the data files --- .../tabs/gsas2/call_G2sc.py | 3 --- .../tabs/gsas2/model.py | 22 +++++++++++-------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py index a4276f37517e..de7d0ab2de27 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/call_G2sc.py @@ -33,9 +33,6 @@ def add_histograms(data_filenames, project, instruments, number_regions): if len(instruments) != 1: raise ValueError("You must provide only one instrument file.") - if number_regions <= 1: - raise ValueError("The data file must contain multiple banks.") - # Add histograms for each bank within that file for bank_index in range(1, number_regions + 1): # GSAS-II uses 1-based indexing project.add_powder_histogram( diff --git a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py index 4754b7ba3311..cd0318500a20 100644 --- a/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py +++ b/qt/python/mantidqtinterfaces/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py @@ -460,13 +460,7 @@ def call_subprocess(self, command_string_list: List[str], gsas_binary_paths: Lis shell_output = shell_process.communicate(timeout=self.config.timeout) if shell_process.returncode != 0: - if "not enough values to unpack (expected 2, got 0)" in shell_output[-1]: - # The error message happens when it fails to read inst param file with the same number of histograms - logger.error( - "* Incorrect instrument file. please use the instrument filewith the same number of histograms as the focused data." - ) - else: - logger.error(f"GSAS-II call failed with error: {shell_output[-1]}") + logger.error(f"GSAS-II call failed with error: {shell_output[-1]}") return None return shell_output @@ -609,6 +603,14 @@ def generate_reflections_from_space_group(self) -> None: # X Limits # ========= + def get_no_banks(self, prm_file): + with open(prm_file) as f: + for line in f.readlines(): + if "BANK" in line and len(line.split()) == 3: + return int(line.split()[-1]) + + return -1 + def understand_data_structure(self) -> None: if len(self.file_paths.instrument_files) != 1: logger.error("* You must provide exactly one instrument file.") @@ -624,8 +626,10 @@ def understand_data_structure(self) -> None: file_bank_count = loop_focused_workspace.getNumberHistograms() banks_per_file.append(file_bank_count) - if file_bank_count <= 1: - logger.error("* Each data file must contain multiple banks.") + no_banks = self.get_no_banks(self.file_paths.instrument_files[0]) + + if file_bank_count != no_banks: + logger.error("* All data files should have the same number of banks as the instrument file.") return False for workspace_index in range(file_bank_count):