Ticket #9052 (closed: fixed)
IntegrateEllipsoids on all instruments
Reported by: | Owen Arnold | Owned by: | Owen Arnold |
---|---|---|---|
Priority: | blocker | Milestone: | Release 3.4 |
Component: | Diffraction | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | #11307 | Tester: | VickieLynch |
Description (last modified by Nick Draper) (diff)
IntegrateEllipsoids should work on all instruments.
Pascal has reiterated that this is very important to get sorted during this development cycle. This could be down to this ongoing rectangular detector issue.
Change History
comment:2 Changed 7 years ago by Nick Draper
- Owner changed from Anyone to Peter Peterson
- Status changed from new to infoneeded
- Component changed from Framework to Diffraction
- Description modified (diff)
- Blocking 8930 added
comment:3 Changed 7 years ago by Nick Draper
- Owner changed from Peter Peterson to Owen Arnold
Can you add a sample case where this is not working?
comment:4 Changed 7 years ago by Owen Arnold
Doesn't work because WISH is collecting Histogram data due to volume. Does not match input restriction for EventWorkspace.
comment:6 Changed 7 years ago by Nick Draper
- Status changed from new to assigned
- Owner changed from Owen Arnold to Peter Peterson
Pete, feel free to assign onward in your team.
comment:9 Changed 6 years ago by Peter Peterson
- Milestone changed from Release 3.3 to Release 3.4
I cannot get this done before the release.
comment:11 Changed 6 years ago by Owen Arnold
- Status changed from assigned to inprogress
refs #9052. Create test data.
This is extremely hard to do, which is presumably why this algorithm never had any unit tests in the first place.
Changeset: 5b1edf516271da8c7c4885bbc343d301b3a4887e
comment:12 Changed 6 years ago by Owen Arnold
refs #9052. Upgrade CreateSampleWorkspace
Add options to move the banks and set the pixel spacing for the fake instrument.
This is an important feature of working with SCD data, where we want to move the bank in such a way that we have coverage for specific reflections. I've unit tested the two new properties I've added as part of this. The default behaviour will be exactly the same as before.
Changeset: c08541f30af7839312cb7176eedc9285ed9ee9c5
comment:13 Changed 6 years ago by Owen Arnold
refs #9052. Get events tests working.
Changeset: 4da84e4f26eeeb9da66d5017bcdbabef4f4fe98e
comment:14 Changed 6 years ago by Owen Arnold
refs #9052. Simplify test setup
Also changed the getPeakShape public method to be const, as this is the right sort of behaviour as the PeakShape should be immutable and should not cause internal change to Peak.
Changeset: 2ad2513054e33da8b2cccf690145ad20cf1e85de
comment:15 Changed 6 years ago by Owen Arnold
refs #9052. Check principle axis
I've added some peak by peak checks using the PeakShape to determine what the principle axis used for integration has been. Since I artifically construct the ellipsoids in the test, I know that they have a principle axis directly along the TOF, and there is only one detector involved. We should therefore expect that the principle axis of the ellipsoid is exactly the same as the scattering qDir. Normalising both before comparison takes out the wavenumber dependency.
I've also started adding in the regression tests needed for the Histogram workspace cases.
Changeset: 4c514006f4275bb9684087fe32a39ca30aa743de
comment:16 Changed 6 years ago by Owen Arnold
refs #9052. Working for Workspace2D
My regression tests now pass for both the EventWorkspace case and the Workspace2D case. These are happy path at this point, but they are doing some quite detailed checks and since the test data in both cases are inter-dependent, I know that they are also producing the same output.
There's performance work still yet to do, and I want to probe the behaviour a little deeper, perhaps with some destructive tests thrown in too.
Conflicts:
Code/Mantid/Framework/MDEvents/src/IntegrateEllipsoids.cpp
Changeset: 342798a091072b9e954580b00d0fde9ce9fc0c95
comment:17 Changed 6 years ago by Owen Arnold
refs #9052. Add destructive and perf tests
Add performance tests prior to optimisation. Add destructive test cases to functional tests.
Changeset: 1fed34dd46bfd93e6930d27e505bb9dd760ee1d6
comment:18 Changed 6 years ago by Owen Arnold
refs #9052. Fix from conflict.
Also update some of the broken documentation.
Changeset: cf6689f2366105d16ced618583adca0736e56925
comment:19 Changed 6 years ago by Owen Arnold
- Status changed from inprogress to verify
- Resolution set to fixed
This is being verified as pull request #317.
comment:20 Changed 6 years ago by Owen Arnold
refs #9052. Fix typdef ambiguity.
Changeset: 42716deb217a252d8a338f942d5d997f6f62682f
comment:21 Changed 6 years ago by Owen Arnold
jenkins test this please
comment:22 Changed 6 years ago by Owen Arnold
refs #9052. Fix accessibility issue.
Changeset: 8329ab7a727a67c505ec768ec4add90ad3a45393
comment:23 Changed 6 years ago by Owen Arnold
Jenkins retest this please
comment:24 Changed 6 years ago by Owen Arnold
Jenkins retest this please
comment:25 Changed 6 years ago by Owen Arnold
Jenkins retest this please
comment:26 Changed 6 years ago by Owen Arnold
refs #9052. Merge in master.
System test changes and others to include in feature branch.
Merge branch 'master' into feature/9052_integrate_ellipsoids
Changeset: 6095d7d60e4fad8187e9489e4d5395f4872be285
comment:27 Changed 6 years ago by Owen Arnold
jenkins retest this please
comment:28 Changed 6 years ago by Owen Arnold
jenkins retest this please
comment:29 Changed 6 years ago by Owen Arnold
Tester, look at the failed test. It's just the windows build where one of the doc tests has spuriously failed.
comment:31 Changed 6 years ago by VickieLynch
- Status changed from verifying to closed
- Tester set to VickieLynch
Merge pull request #317 from mantidproject/feature/9052_integrate_ellipsoids
Make IntegrateEllipsoids universal
Full changeset: 5af7ec03debfc1a40cc03cf527e400d2d3dcef16
comment:33 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 9895