Ticket #6056 (closed: fixed)

Opened 8 years ago

Last modified 5 years ago

Put back 'bug' into SmoothNeighbours

Reported by: Peter Peterson Owned by: Owen Arnold
Priority: critical Milestone: Release 2.3
Component: Mantid Keywords:
Cc: Blocked By: #6067
Blocking: Tester: Peter Peterson

Description

In #6025, the feature added to check for rectangular detectors added in ab8711aa82f879f22866fc3dbe53a7cb055cdf2b disabled a feature which was smoothing data based on the instrument hierarchy rather than the distance for eight-packs like NOMAD.

This was working during the last release before the change.

Change History

comment:1 Changed 8 years ago by Owen Arnold

refs #6056. Group properties

Changeset: 5f31871726c3fb7288fb63de375ca4ae747f2a0c

comment:2 Changed 8 years ago by Owen Arnold

refs #6056. Now works according to groupings.

Changeset: 016a91334064bcfabddb0d0e214264542c0a6159

comment:3 Changed 8 years ago by Owen Arnold

Here is the proposal for the fix suggested by Peter:

  1. Create a second "propertyGroup" for the radius based parameters: RadiusUnits, Radius, NumberOfNeighbours, and SumNumberOfNeighbours
  1. Make sure that people don't specify parameters in both property groups
  1. Add a log.information for whether it is doing "ubiquitious" or "rectangular" mode

One other small change: The second group shouldn't be called "Rectangular Detectors Only" when we make it work with adjacent pixels that are rectangle-like. For now I would just change the group label to "Rectangular Detectors"

comment:4 Changed 8 years ago by Owen Arnold

  • Blocked By 6067 added

comment:5 Changed 8 years ago by Owen Arnold

  • Status changed from new to accepted
  • Tester set to Peter Peterson

comment:6 Changed 8 years ago by Owen Arnold

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

Peter understands how this should be working as he helped scope the requirements.

comment:7 Changed 8 years ago by Owen Arnold

comment:8 Changed 8 years ago by Peter Peterson

  • Status changed from verify to verifying

comment:9 Changed 8 years ago by Peter Peterson

  • Status changed from verifying to closed

I think that the tests of properties being in certain groups is a little odd, but the work was done correctly. Thanks for moving so fast on this one.

comment:10 Changed 8 years ago by Owen Arnold

Yes, I agree it is a little odd. But if people start messing around with the groups to which the properties belong to, this could break the functionality I just introduced. At least with the unit tests on the groups the unit test will break and indicate that things have changed.

comment:11 Changed 8 years ago by Owen Arnold

refs #6056. Group properties

Changeset: 5f31871726c3fb7288fb63de375ca4ae747f2a0c

comment:12 Changed 8 years ago by Owen Arnold

refs #6056. Now works according to groupings.

Changeset: 016a91334064bcfabddb0d0e214264542c0a6159

comment:13 Changed 8 years ago by Owen Arnold

refs #6056. Group properties

Changeset: 5f31871726c3fb7288fb63de375ca4ae747f2a0c

comment:14 Changed 8 years ago by Owen Arnold

refs #6056. Now works according to groupings.

Changeset: 016a91334064bcfabddb0d0e214264542c0a6159

comment:15 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 6902

Note: See TracTickets for help on using tickets.