Ticket #9802 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Updating value of ParameterMap copy updates the original too

Reported by: Martyn Gigg Owned by: Martyn Gigg
Priority: critical Milestone: Release 3.2
Component: Framework Keywords:
Cc: Blocked By:
Blocking: Tester: Roman Tolchenov

Description

This is most easily demonstrated using the script below. The data is in the AutoTest/UsageData directory.

# load data
data = Load("HRP39180")
# Move the detector so we can see affect of calibration
MoveInstrumentComponent(data, ComponentName="HRPD",X=0.0,Y=0.0,Z=1.0,RelativePosition=True)
print "data detector pos:",data.getDetector(0).getPos()

# load calibration
Load("HRP38094Calib", OutputWorkspace="calibration")
calib_det_pos = mtd['calibration'].getDetector(0).getPos()
print "calibration detector pos:",calib_det_pos

# copy parameters from calibration to data
CopyInstrumentParameters("calibration", data)
print "data detector pos after calib:",data.getDetector(0).getPos() # should match statment from calibration workspace

# Now move component on data workspace, where that component was a detector in the calibrated workspace and the calibration position will move
MoveInstrumentComponent(data, DetectorID=1100,X=0.0,Y=0.0,Z=5.0,RelativePosition=False)
print "data detector pos after move:",data.getDetector(0).getPos() # should match statment from calibration workspace
calib_det_pos_end = mtd['calibration'].getDetector(0).getPos()
print "calibration detector after move:",calib_det_pos_end
if calib_det_pos_end != calib_det_pos:
        raise RuntimeError("Calibrated detector positions have moved!")

Change History

comment:1 Changed 6 years ago by Martyn Gigg

  • Status changed from new to assigned

comment:2 Changed 6 years ago by Martyn Gigg

  • Summary changed from ParameterMap copy constructor results in shared parameters to Updating value of ParameterMap copy updates the original too

comment:3 Changed 6 years ago by Martyn Gigg

  • Status changed from assigned to inprogress

Change private method name

Avoids using get for something that is not really a constant. Refs #9802

Changeset: 1351e600c7ab7d7b0d3479944438b52e8b48806d

comment:4 Changed 6 years ago by Martyn Gigg

Add a set of tests to define the behaviour that is currently broken.

Refs #9802

Changeset: e4616a87522e3f83941a621563e6e32871cd0356

comment:5 Changed 6 years ago by Martyn Gigg

Fix add/replace of values for generic types

A brand new parameter object is always created and it either overwrites an existing value or adds to the map. The addPositionCoordinate/addRotationParam are still broken. Refs #9802

Changeset: 395dbcccb948c3c2b6f4f7f7cf6c0907d693f839

comment:6 Changed 6 years ago by Martyn Gigg

Fix addPositionCoordinate for copied objects.

Refs #9802

Changeset: b3bfbc74336ee2f54075dac9329d670c3462fc51

comment:7 Changed 6 years ago by Martyn Gigg

Fix addRotationParam for copied objects.

Refs #9802

Changeset: b2fb511fad821dd0ec3255deb0485993000399bb

comment:8 Changed 6 years ago by Martyn Gigg

System test to cover reusing calibration workspaces

Refs #9802

Changeset: 4e994619172df148652406d1d64678950e1b26b1

comment:9 Changed 6 years ago by Martyn Gigg

Remove unused private method ParameterMap::retrieveParameter.

Refs #9802

Changeset: 626b4b338f13659e8da99dc8f43b98e0af1b3756

comment:10 Changed 6 years ago by Martyn Gigg

Add a test for the paramter values after copying between maps.

Refs #9802

Changeset: 3d41c5fa42001d72c2fff2a49654d506fb4865a0

comment:11 Changed 6 years ago by Martyn Gigg

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

Branch: bugfix/9802_parameter_map_copy_and_update in mantid repository AND bugfix/9802_parameter_map_copy_and_update in systemtests

Tester: The script in the description should run to completion without error. The new systemtest should be passing and there are new unit tests for this behaviour. Please check the code carefully as this is a core class.

comment:12 Changed 6 years ago by Roman Tolchenov

  • Status changed from verify to verifying
  • Tester set to Roman Tolchenov

comment:13 Changed 6 years ago by Roman Tolchenov

  • Status changed from verifying to reopened
  • Resolution fixed deleted

During testing it was noticed that comparison of V3D objects in python is a bit wrong: != operator apparently compares references, not values. Add this code to the above script to demonstrate the issue:

print repr(calib_det_pos)
print repr(calib_det_pos_end)

print calib_det_pos_end == calib_det_pos
print calib_det_pos_end != calib_det_pos
print calib_det_pos_end - calib_det_pos
print type(calib_det_pos)

comment:14 Changed 6 years ago by Martyn Gigg

  • Status changed from reopened to inprogress

comment:15 Changed 6 years ago by Martyn Gigg

Add != Python operator for V3D & VMD.

This ensures that compairsons are done by value and not object identity. Refs #9802

Changeset: 2397e292d0e01f72604c9ddcb4edfed78201464f

comment:16 Changed 6 years ago by Martyn Gigg

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

comment:17 Changed 6 years ago by Roman Tolchenov

  • Status changed from verify to verifying

comment:18 Changed 6 years ago by Roman Tolchenov

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/bugfix/9802_parameter_map_copy_and_update'

Full changeset: 04cec61f45ca8ed841a007977555032c94c49a78

comment:19 Changed 6 years ago by Roman Tolchenov

Merge remote-tracking branch 'origin/bugfix/9802_parameter_map_copy_and_update'

Full changeset: 357e2c4e6903db4f021b35bf31892d59b698e7c9

comment:20 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 10644

Note: See TracTickets for help on using tickets.