pace-neutrons / Horace

Horace is a suite of programs for the visualization and analysis of large datasets from time-of-flight neutron inelastic scattering spectrometers.
https://pace-neutrons.github.io/Horace/stable/
GNU General Public License v3.0
7 stars 4 forks source link

Problem with setting instrument and sample in gen_sqw #712

Open cmarooney-stfc opened 2 years ago

cmarooney-stfc commented 2 years ago

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 calcsqw (line 47) In rundatah/calc_sqw (line 137) In rundata_write_tosqw (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 commented 2 years ago

@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 commented 2 years ago

Hi Alex

Thanks for this. As the code is patchy, I will fix so it works for me and you can review it in my PR when that happens

Cheers Chris

From: abuts @.> Sent: 02 September 2021 18:31 To: pace-neutrons/Horace @.> Cc: Marooney, Christopher (STFC,RAL,SC) @.>; Author @.> Subject: Re: [pace-neutrons/Horace] Problem with setting instrument and sample in gen_sqw (#712)

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

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/pace-neutrons/Horace/issues/712#issuecomment-911906896, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AQ34IRD247OQS67KPQP7PPDT76YDZANCNFSM5DJPEBZA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

This email and any attachments are intended solely for the use of the named recipients. If you are not the intended recipient you must not use, disclose, copy or distribute this email or any of its attachments and should notify the sender immediately and delete this email from your system. UK Research and Innovation (UKRI) has taken every reasonable precaution to minimise risk of this email or any attachments containing viruses or malware but the recipient should carry out its own virus and malware checks before opening the attachments. UKRI does not accept any liability for any losses or damages which the recipient may sustain due to presence of any viruses.