mantid
mantid copied to clipboard
Add GSASII tab to EngDiff interface
Note this includes a quick fix for issue #34262, but worth checking that issue isn't more complex.
To test:
You may wish to test this on a development IDAaaS environment as this will have gsas2 installed.
Data to test is in babylon>Public>Daniel Murphy>GSASII tab. Note the CIF file is from the Mantid code base and the instrument and focused data for the tests below can be generated by running the Calibration and Focus tabs (and they will prefill the GSASII tab):
# Calibration
/cycle_19_1/ENGINX00305738.nxs
# Sample Run
/cycle_19_1/ENGINX00305761.nxs
# Vanadium
/cycle_19_1/ENGINX00307521.nxs
Test 1: Bank 1
# Instrument Group
ENGINX_305738_bank_1.prm
# Phase
FE_GAMMA.cif
# Focused Data
ENGINX_305761_307521_bank_1_TOF.gss
Test 2: All banks
# Instrument Group
ENGINX_305738_all_banks.prm
# Phase
FE_GAMMA.cif
# Focused Data
ENGINX_305761_307521_all_banks_TOF.gss
Test 3: All banks, separate Focused files
# Instrument Group
ENGINX_305738_all_banks.prm
# Phase
FE_GAMMA.cif
# Focused Data
ENGINX_305761_307521_bank_1_TOF.gss, ENGINX_305761_307521_bank_2_TOF.gss
Test 4: All banks, separate Focused and Instrument files
# Instrument Group
ENGINX_305738_bank_1.prm, ENGINX_305738_bank_2.prm
# Phase
FE_GAMMA.cif
# Focused Data
ENGINX_305761_307521_bank_1_TOF.gss, ENGINX_305761_307521_bank_2_TOF.gss
(I currently don't want to allow one focused data file with many instrument files)
I have added reitveld data from the GSASII tutorial on babylon but running those files will not work as my assumptions have focused on the output file types from the EngDiff interface.
- When you have run a refinement you can change the fitting range by dragging the start and end X lines or by altering the min and max line edits. If you refine again with the same load parameters then the new fitting range will be used. In any of the all bank examples you will refine 2 spectra, but only the first is plotted, to plot the second toggle the combobox just above the plot. For me, the best fit for the examples above was setting the override cell length to 3.65.
Fixes #34124
Not yet added release notes or docs!! This does not require release notes because fill in an explanation of why If you add release notes please save them as a separate file using the Issue or PR number as the file name. Check the file is located in the correct directory for your note(s).
Reviewer
Please comment on the following (full description):
Code Review
- Is the code of an acceptable quality?
- Does the code conform to the coding standards?
- Are the unit tests small and test the class in isolation?
- If there is GUI work does it follow the GUI standards?
- If there are changes in the release notes then do they describe the changes appropriately?
- Are the release notes saved in a separate file, using Issue or PR number for file name and in the correct location?
Functional Tests
- Do changes function as described? Add comments below that describe the tests performed?
- Do the changes handle unexpected situations, e.g. bad input?
- Has the relevant (user and developer) documentation been added/updated?
Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers
will take care of it.
I have tried to build a linux package from this branch which would aide testing on IDAaaS: https://builds.mantidproject.org/job/build_packages_from_branch/77/
I would note that I also had on my list to add tests for call_G2sc.add_histogram() and EnggUtils._save_output_files() (to check that SaveGSS has append=False). These are hopefully not needed for this PR to be merged before code freeze.
I've reviewed this PR and have tested on Windows and Ubuntu and it works fine. There are possibly a couple of outstanding pieces of work that can be picked up on separate PRs and I'll discuss with Dan when he's back from holiday:
- ~~testing on IDAaaS. I haven't done this as part of my review but would like to check if Dan's done any of this~~
- documentation (docs\source\interfaces\diffraction\Engineering Diffraction.rst). It's possible that it makes sense to do this once the UI has had some further use by Saurabh and the ENGINX team to avoid having to update screenshots multiple times
Just tried testing this on IDAaaS (using .tar.xz found here https://builds.mantidproject.org/job/build_packages_from_branch/85/) I got the following error when trying to refine the bank 1 data
Line not defined: 52832.78540 0.00000000 0.00000000
Line not defined: END
Error in execution of algorithm LoadGSS:
All arguments to init must be positive and non-zero
Traceback (most recent call last):
File "/home/rw1061637/Downloads/mantidworkbenchunstable/lib/python3.8/site-packages/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/presenter.py", line 36, in on_refine_clicked
number_output_histograms = self.model.run_model(load_params, refine_params, project_name,
File "/home/rw1061637/Downloads/mantidworkbenchunstable/lib/python3.8/site-packages/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py", line 94, in run_model
self.evaluate_further_inputs(user_limits)
File "/home/rw1061637/Downloads/mantidworkbenchunstable/lib/python3.8/site-packages/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py", line 138, in evaluate_further_inputs
if not self.loop_phase_files() or not self.validate_x_limits(user_x_limits):
File "/home/rw1061637/Downloads/mantidworkbenchunstable/lib/python3.8/site-packages/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py", line 512, in validate_x_limits
self.understand_data_structure()
File "/home/rw1061637/Downloads/mantidworkbenchunstable/lib/python3.8/site-packages/mantidqtinterfaces/Engineering/gui/engineering_diffraction/tabs/gsas2/model.py", line 503, in understand_data_structure
loop_focused_workspace = LoadGSS(Filename=input_file, OutputWorkspace="GSASII_input_data")
File "/home/rw1061637/Downloads/mantidworkbenchunstable/lib/python3.8/site-packages/mantid/simpleapi.py", line 1058, in __call__
algm.execute()
IndexError: All arguments to init must be positive and non-zero
To Reproduce (may not need all these steps but this is what I did): (1) Load .prm in calibration tab (auto-filled in GSAS tab nicely!) - this produced the following error
Unable to load calibration file /babylon/Public/DanielMurphy/GSASII Tab/ENGINX_305738_bank_1.nxs. Error: Problem setting "Filename" in Load: Invalid value for property Filename (list of str lists) from string "/babylon/Public/DanielMurphy/GSASII Tab/ENGINX_305738_bank_1.nxs": When setting value of property "Slave": File "/babylon/Public/DanielMurphy/GSASII Tab/ENGINX_305738_bank_1.nxs" not found
If the file has been found but you got this error, you might not have read permissions or the file might be corrupted.
If the file has not been found, you might have forgotten to add its location in the data search directories.
Even though I pointed it at the .prm
.
It loaded the following workspaces (which I think look correct)
(2) Switch to GSAS II tab
(3) fill in project name
(4) Set paths the phase and data
(5) Change refinement setting as so
(6) Click Refine in GSAS II
Did I do something wrong?
I've done some testing on IDAaaS with a new build that contains all of the fixes up to and including 1st Sept: https://builds.mantidproject.org/job/build_packages_from_branch/93/
There seems to be a problem relating to the subprocess.POpen command. On IDAaaS it removes the curly brackets at the start and end of the JSON string that is included in the command_string variable. I've just checked on Ubuntu and the latest code still works on there so I think this is a RHEL only problem. There are some comments online about POpen doing this although not specifically on RHEL7 - the comments suggest that the "{" and "}" characters have a particular meaning in certain shells and get removed.
I was able to make some changes to the .py files installed to debug the problem and to try out a few alternatives. It seems to be the case that putting single quotes around the JSON string stops the curly brackets being removed. I also had to remove the code in gsas2/model.py to replace "
with \\"
. The POpen call continues to work on Ubuntu with this change but it doesn't work on Windows.
I will push a change calling POpen with shell=False and see if that helps. The default shell on different OS's may behave slightly differently so hopefully this approach will eliminate that source of variability.
Did I do something wrong?
I think the first error about not being able to find a .nxs file is because the file doesn't exist on Dan's babylon share. The calibration tab is looking for a table workspace containing the calibration parameters (difc etc). I think he was probably expecting us to go straight to the GSAS2 tab and load the .prm file there so he didn't upload all of the calibration outputs onto the share. As discussed the LoadGSS problem is perhaps because the format of the bank 1 .gss file on Dan's share has a mistake in it. The BANK line has a # character at the start:
# BANK 1 10186 10186 RALF 171834 39 171834 0.00023 FXYE