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

Problem with setting instrument and sample in gen_sqw #712

Open
cmarooney-stfc opened this issue Sep 2, 2021 · 2 comments
Open

Problem with setting instrument and sample in gen_sqw #712

cmarooney-stfc opened this issue Sep 2, 2021 · 2 comments
Assignees
Labels
Discussion Task requiring further discussion MVP Wanted for MVP

Comments

@cmarooney-stfc
Copy link
Collaborator

Hi @abuts Alex - This is a request to check on some code. Could you see whether my diagnosis below is correct? Happy to do any actual changes myself once it is clear if there is a problem. But asking if you can help look at this first. Thanks, Chris

Running runtests test_gen_sqw_accumulate_sqw_nomex::test_accumulate_and_combine1to4 in test_gen_sqw_workflow.
Reaching the point (from dbstack)
In gen_sqw>convert_to_tmp_files (line 843)
In gen_sqw (line 460)
In accumulate_sqw (line 136)
In gen_sqw_accumulate_sqw_tests_common/test_accumulate_and_combine1to4 (line 523)

Before this, at line 412 in gen_sqw.m, if the if-clause is followed because accumulate_old_sqw==F and nindx==1, run_files{1}.instrument and .sample are set from the instrument and sample arrays (presumed fine as far as I can see. This is OK and then later at the point
In calc_sqw_ (line 47)
In rundatah/calc_sqw (line 137)
In rundata_write_to_sqw_ (line 63)
In gen_sqw_files_job.runfiles_to_sqw (line 214)
In gen_sqw>convert_to_tmp_files (line 843)
In gen_sqw (line 460)
In accumulate_sqw (line 136)
In gen_sqw_accumulate_sqw_tests_common/test_accumulate_and_combine1to4 (line 523)
calc_sqw_data_and_header is called and this sets header.instrument and header.sample from obj.instrument and obj.sample, where obj is run_files{1}. All this looks OK and provides a template for how this code should work.

However if at line 412 in gen_sqw.m, nindx ~=1 (in this case it is 2) then execution switches to the else at L.433 and then to the else at L.447 which reduces instrument and sample to the used parts of these arrays. However the values are not copied into the relevant run_files objects, and so the instruments and samples in the run_files are set up from the defaults and are currently empty structs.

So for the case accumulate_old_sqw=F and nindx>1 there seems to be a problem. The case accumulate_old_sqw==T has not yet been investigated but I would suspect that another different treatment is required.

@abuts
Copy link
Member

abuts commented Sep 2, 2021

@cmarooney-stfc
Hi, Chris,
Despite its the old code, it never have been used for generating sqw file with instrument and sample provided.
As the result, the code, which uses instrument and sample is patchy.

the instrument and sample were set up later when you want to analyze resolution.
This is why the code, which uses/sets-up instrument and sample is patchy and may be incorrect.

I am not sure if it knows what to do with instrument and sample array provided as input as currently only one instrument and sample allowed.

The logic of the code have to be modified if you want to provide instrument and sample for gen_sqw

The main modification is what to do with array of instruments and samples. It should be only one, but the generation logic assumes it may be many.

The algorithm, to deal with this should be developed and is completely in scope of your experiment info modifications

@cmarooney-stfc
Copy link
Collaborator Author

cmarooney-stfc commented Sep 3, 2021 via email

@oerc0122 oerc0122 added Discussion Task requiring further discussion MVP Wanted for MVP labels Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Task requiring further discussion MVP Wanted for MVP
Projects
None yet
Development

No branches or pull requests

3 participants