Make IntegrateEllipsoids universal #317
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is extremely hard to do, which is presumably why this algorithm never had any unit tests in the first place.
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.
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.
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.
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
Add performance tests prior to optimisation. Add destructive test cases to functional tests.
Also update some of the broken documentation.
OwenArnold
added
High Priority
|
jenkins test this please |
|
Jenkins retest this please |
2 similar comments
|
Jenkins retest this please |
|
Jenkins retest this please |
System test changes and others to include in feature branch. Merge branch 'master' into feature/9052_integrate_ellipsoids
|
jenkins retest this please |
1 similar comment
|
jenkins retest this please |
|
Tester, look at the failed test. It's just the windows build where one of the doc tests has spuriously failed. |
VickieLynch
added a commit
that referenced
this pull request
Mar 9, 2015
…ipsoids Make IntegrateEllipsoids universal
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
IntegrateEllipsoids doesn't work on MatrixWorkspaces of type Workspace2D. This is a big problem, because for many instruments at ISIS, runs are collected in histogram mode not event mode. We have a big drive to make the the integration routines universal, and to verify them cross facility and against existing programs such as SXD2001.
IntegrateEllipsoids does not currently have any unit tests (at least on the algorithm directly), so that must be the starting point for this work.
Tester : Check the code. All new and adapted unit tests should work.
See #9052