Ticket #10524 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Location parameters mutate

Reported by: Harry Jeffery Owned by: Harry Jeffery
Priority: critical Milestone: Release 3.3
Component: Framework Keywords:
Cc: anders.markvardsen@… Blocked By:
Blocking: Tester: Federico M Pouzols

Description

To reproduce:

Load(Filename='/PATH/TO/systemtests/Data/MAPS00018314.nxs', OutputWorkspace='data', LoaderName='LoadISISNexus', LoaderVersion=2)
SaveParameterFile("data","orig_params.xml",True)
ClearInstrumentParameters("data", True)
LoadParameterFile("data", "orig_params.xml")
SaveParameterFile("data","new_params.xml",True)

orig_params.xml and new_params.xml will have different position parameters for a lot of components.

Attachments

orig_params_one_detector.xml (465 bytes) - added by Anders Markvardsen 6 years ago.
Parameter file with location parameters for one detector

Change History

comment:1 Changed 6 years ago by Martyn Gigg

  • Status changed from new to assigned

Changed 6 years ago by Anders Markvardsen

Parameter file with location parameters for one detector

comment:2 Changed 6 years ago by Anders Markvardsen

When running

Load(Filename='MAPS00018314.nxs', OutputWorkspace='data')
ClearInstrumentParameters("data", True)
LoadParameterFile("data", r"C:\Backup\Backup_folder1\work\code\Mantid\DefaultMantidOutput\orig_params_one_detector.xml")
SaveParameterFile("data", r"C:\Backup\Backup_folder1\work\code\Mantid\DefaultMantidOutput\new_params_one_detector.xml")

from a freshly opened Mantidplot I find that the returned output "new_params_one_detector.xml" contains nothing.

However then going into MantidPlot a running manually SaveParameterFile I got

<?xml version="1.0" encoding="UTF-8"?>
<parameter-file instrument="MAPS" valid-from="1900-01-31T23:59:59">
	<component-link id="11103166" name="MAPS/A1_window/PSD_TUBE_STRIP 8 pack up/PSD_TUBE_STRIP/PSD_TUBE_STRIP pixel">
		<parameter name="x">
			<value val="3.51305"/>
		</parameter>
		<parameter name="y">
			<value val="-6.20741"/>
		</parameter>
		<parameter name="z">
			<value val="11.6718"/>
		</parameter>
	</component-link>
</parameter-file>

which does not agree with orin_params_one_detector.xml at all.

Prior to writing this Harry and I ran another example where the output to "new_params_one_detector.xml" was different hence it appears that the output to "new_params_one_detector.xml" depends on what was done previous to the workspace in terms of save, load and clear.

comment:3 Changed 6 years ago by Harry Jeffery

  • Status changed from assigned to inprogress

comment:4 Changed 6 years ago by Harry Jeffery

Refs #10524 Translate existing pos parameters...

... instead of creating new ones from the position. Seems like generating new ones is wildly inaccuarate.

Changeset: fb3dc00e34c32fb80a8df0a4735015546392701a

comment:5 Changed 6 years ago by Harry Jeffery

  • Status changed from inprogress to verify
  • Resolution set to fixed

Testing

  • Run the script given in the ticket's description.
  • orig_params.xml and new_params.xml should be semantically the same (i.e. same data), but may be syntactically different (i.e. parameters in a different order).
  • Try the same process with the parameter file Anders gave.

Note: There are precision errors when saving location parameters. For example: -0.527492 may become -0.52741999999996. The error appears before the algorithm gets to the data, so there's nothing it can do about it, and the difference is so small it's negligible.

comment:6 Changed 6 years ago by Federico M Pouzols

  • Status changed from verify to verifying
  • Tester set to Federico M Pouzols

comment:7 Changed 6 years ago by Federico M Pouzols

  • Status changed from verifying to closed

This is working well now. I tried the script given in the ticket description and I got identical results in the orig and new parameter files. There are plenty of parameters in different order for the component MAPS (in new_params.xml the parameters are sorted alphabetically), but both files are semantically identical. I (actually kdiff3) didn't observe any precision errors/differences.

Then I tried with Anders' one_detector file. I run this sequence of commands:

Load(Filename='/home/fedemp/mantid-repos/systemtests/Data/MAPS00018314.nxs', OutputWorkspace='data', LoaderName='LoadISISNexus', LoaderVersion=2)
ClearInstrumentParameters("data", True)
LoadParameterFile("data", "orig_params_one_detector.xml")
SaveParameterFile("data","new_params_one_detector.xml",True)

And I get exactly the same results. This time there are visible precision differences. Anders' orig_params_one_detector.xml​ uses 5 decimal digits whereas my MantidPlot saves with 16 decimal digits and generates erros within +-4e-16. This seems to depend on local setup/libraries.

I think that this issue is solved. But as Anders noticed, there may be other glitches (or unclear behavior) related to the load/save history of the data and/or instrument. The one_detector example works well if you start from a fresh MantidPlot. But if you've previously loaded another parameters file what you will get is the full ~60MB parameters file where the x,y,z values of component id="11103166" have been replaced with the numbers used in "orig_params_one_detector.xml​". If I clear the instrument before loading, then what I get is an empty instrument parameters file. In a way, this makes sense to me, but just to note it here in case further investigation or tickets are required.

comment:8 Changed 6 years ago by Federico Montesino Pouzols

Merge remote-tracking branch 'origin/feature/10524_location_params_mutate'

Full changeset: 6363ee9866d929e687c59311856ddd87932bb640

comment:9 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 11366

Note: See TracTickets for help on using tickets.