Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting rundatah par file, reading instrument info - issues #1769

Open
cmarooney-stfc opened this issue Oct 31, 2024 · 1 comment
Open

Setting rundatah par file, reading instrument info - issues #1769

cmarooney-stfc opened this issue Oct 31, 2024 · 1 comment

Comments

@cmarooney-stfc
Copy link
Collaborator

cmarooney-stfc commented Oct 31, 2024

(1) The constructor of loader_nxspe (loader_nxspe.m) includes a method to read instrument info (read_instrument_info, L.268ff). Instrument reading with these methods happens from the set.par_file_name method of rundata if the data file name (obj.file_name) is not set. The code after L.274 assumes that the file is an HDF5 file in that (a) at L.277 H5 info tries to be read from the file and (b) before that, find_root_nexus_dir is called (L.275) which also calls h5info as its first substantial action. However in test_transformation and (looking at CI failures) many other tests, the file is a file equivalent of detpar, in this case 96dets.par.

h5info fails; however, the error is ignored because nxspe reads are set up to ignore incomplete instrument data. Something should be done to ensure that these non-H5 par files are appropriately handled. It is not at all obvious that receiving a detpar file should lead to handling a full instrument reconstruction

(2) In this case, read_instrument_info is called from loader_nxspe during call by rundata to set the par file name. In set.par_file_name, if the loader does not exist it will be created. It is unclear why the loader should be created at this point.

@abuts
Copy link
Member

abuts commented Nov 1, 2024

Current idea of reading various format data file is that you define a composite loader which consists of parameter loader and data loader. Each is selected on the basis of the file name proivided -- usually to constructor. Currently supported file formats are ascii(*.par and *.phs) and nxspe for detectors and ascii(*.spe) and nxspe for data.

This way of interpreting different file formats is a frajile one.

The solution of the particular problem of course can be performed by introducing additional checks to the file name setter, but this would add additional complexity to already complex code. Proper way of dealing with it (suggested earlier) would be introduction of two file format factories for data and instrument parameters loaders, deleting composite loader and placing task of combining data and instrument information together to rundata class.

data loaders factory would deal with classes (loaders) which have interface allowing to get signal, error and energy binning (with optinal 2-theta not used by us but may be necessary for somebody else (some spe files (produced in ILL?) contain correct theta so that all information necessary for powder analysis can be obtained from *.spe).

instrument parameters loaders factory would deal with classes which load detectors positions (all of them) and optional instruments. Would it be different class or loader retuning empty instrument if instrument info is missing is unimportant implementation details which may be decided later.

Then, when you provide proper filenames to rundata class instance, filename setters would talk to appropriate factories and select appropriate loaders for the data. The code will become clear and easy unit testable so relianle as the result. The code can be easy expanded in a future if other data formats are requested (e.g. Mantid-workspace nxs files)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants