Ticket #10263 (closed: fixed)
Change PoldiFitPeaks1D when close peaks are present
Reported by: | Michael Wedel | Owned by: | Michael Wedel |
---|---|---|---|
Priority: | major | Milestone: | Release 3.4 |
Component: | Diffraction | Keywords: | POLDI |
Cc: | Blocked By: | ||
Blocking: | Tester: | Federico M Pouzols |
Description (last modified by Michael Wedel) (diff)
Currently, PoldiFitPeaks1D refines all peaks independently. Sometimes when two peaks are very close to each other, this does not work properly.
Instead, the algorithm should fit peaks that are only a few FWHMs apart together as a composite function. In that case, the x-ranges have to be combined so that they contain all peaks that are considered "close".
Change History
comment:3 Changed 6 years ago by Michael Wedel
- Status changed from assigned to inprogress
Refs #10263. Fixing constness in PoldiFitPeaks1D
Changeset: e4f146f5dae99c15f1326b27d45e5d8f924169be
comment:4 Changed 6 years ago by Michael Wedel
Refs #10263. Added RefinedRanges helper
Changeset: 3c3dec4f1e04e91387ed668a3e7a56b02abd0434
comment:5 Changed 6 years ago by Michael Wedel
Refs #10263. Changed ranges definition and reducing algorithm
Changeset: eea77dcd04c308c97ab49e583183405ba84b40eb
comment:6 Changed 6 years ago by Michael Wedel
Refs #10623. Changed PoldiFitPeaks1D
The algorithm now tries to find out which Chebyshev-polynomial (as background) gives the best fit.
Changeset: 9e04b3305dc166d8f66442c9851f8f366575c639
comment:7 Changed 6 years ago by Michael Wedel
Refs #10263. Moving modified algorithm.
Since the old algorithm should be preserved, the new one is moved to a new set of files which
Changeset: 09301a40ad43382266a1801547263e0ab374121e
comment:8 Changed 6 years ago by Michael Wedel
Refs #10263. Reverting PoldiFitPeaks1D
Since there's a new version of the algorithm now, version 1 is reverted.
Changeset: fa6cf75d7becbd2b02bb13424d4d21fc644e6076
comment:9 Changed 6 years ago by Michael Wedel
Refs #10263. Adjusting system test for PoldiFitPeaks1D
System test has to be changed because of new parameters, new version of algorithm. Tolerance for difference in position was relaxed slightly.
Changeset: 43b369383f4f28b83f50ca665dc840580e4d880d
comment:10 Changed 6 years ago by Michael Wedel
Refs #10263. Adding new algorithm PoldiPeakSummary
PoldiFitPeaks1D produces a summary table with peak information. Since this will re-occur in the 2D-fit and also in version 2 of the algorithm, this is put into a separate algorithm. It's very small and it will be much easier to deal with possible changes to the output format of poldi peaks.
Changeset: 8706e65aae9f290536931d98960f1283976634c6
comment:11 Changed 6 years ago by Michael Wedel
Refs #10263. Corrected algorithm name for versioning
Changeset: 0896503b93abef4682be25ed4dc4089a64da7e47
comment:12 Changed 6 years ago by Michael Wedel
Refs #10263. First working version of PoldiPeakSummary
Changeset: 9b396adfc096a1d62cdd96d8c831eb5be9f17359
comment:13 Changed 6 years ago by Michael Wedel
Refs #10263. Completing PoldiPeakSummary
Changeset: e2a9105af0577dedd8732ee76d7f52c9b7239dcb
comment:14 Changed 6 years ago by Michael Wedel
Refs #10263. Correcting test for PoldiFitPeaks1D version 2
Changeset: 914eed4f9ae7006bed6723a937c363ba27e2231e
comment:15 Changed 6 years ago by Michael Wedel
Refs #10263. Removing summary code from PoldiFitPeaks1D.
FitCharacteristics is gone too, this is information that could probably go into the log, but it's not necessary to have this information in an output workspace.
Changeset: ba3fdc1398633a19a6173b1c7dc02c80728c31f3
comment:16 Changed 6 years ago by Michael Wedel
Refs #10263. Removed summary part from PoldiFitPeaks1D version 2
Changeset: a77709877b81f8ec8c0510843c13815f48f2a61b
comment:17 Changed 6 years ago by Michael Wedel
- Milestone changed from Release 3.3 to Release 3.4
I am not happy with the current solution although it works as I expected it to. But in fact, v1 is just a special case of v2 and they share a lot of code that is similar but not identical. Before putting considering this "finished" I would like to remove these duplications.
My current plan for this is to clean up version 2 a bit, factoring out the profile generation part, which is the main difference between the two versions now. The code could be shared between both versions, with very few modifications.
I will postpone this to the next release.
comment:18 Changed 6 years ago by Michael Wedel
Refs #10263. Clang-formatting new source code
Changeset: 31b6bc518a2bab566ce4af657bada8270c6b131e
comment:19 Changed 6 years ago by Michael Wedel
Refs #10263. Allowing for a tolerance when merging ranges.
Changeset: 1c6001f0637d738d90111065fa7378b60cd46038
comment:20 Changed 6 years ago by Michael Wedel
Refs #10263. Removing "recursion borders" from PoldiPeakSearch
After careful consideration and inspection of the code and what it does and does not find, I came to the conclusion that this is not needed at all. It caused some strange problems in the past with debug-builds and so I decided it has to go. In fact, if anything, it was cutting down the searched parts of the vector.
Changeset: 0f45e5b204822d8415f3239cb62ec4f5317d9c35
comment:21 Changed 6 years ago by Michael Wedel
Refs #10263. Adjusting peak search system test.
Changeset: 3166885712a9ea3a199f4ab89946405de8582b37
comment:22 Changed 6 years ago by Michael Wedel
Refs #10263. Fixing documentation
Changeset: 9c9f09f373896bbf55588afa6b2771dd4c5c2e3d
comment:23 Changed 6 years ago by Michael Wedel
Refs #10263. Added system test for version 2 of algorithm.
Changeset: 7029c6d8b57caca36b5ea4a8405bf1a160656efd
comment:24 Changed 6 years ago by Michael Wedel
- Status changed from inprogress to verify
- Resolution set to fixed
This is being verified as pull request #125.
comment:25 Changed 6 years ago by Michael Wedel
This is being verified as pull request #10.
comment:26 Changed 6 years ago by Michael Wedel
Refs #10263. Exporting symbols for DLL.
Changeset: a64996a5e1db20123f36b3ea5978be783277735a
comment:27 Changed 6 years ago by Michael Wedel
retest this please
comment:28 Changed 6 years ago by Michael Wedel
Refs #10263. Loop was never executed, but should run at least once.
Changeset: 0c5a17e88987a5ab4f5e3ae0827ba076596dcad8
comment:29 Changed 6 years ago by Michael Wedel
Testing information Make sure all the tests pass, especially the system test "POLDIFitPeaks1DTest", where the new behavior is tested. In systemtests there's a new data file that contains a calculated correlation spectrum - if you like you can plot the output of the following script to see that two peaks are refined together. Increasing the "AllowedOverlap"-parameter may lead to peaks being refined one by one, which should give worse results.
` Load(Filename='poldi_2_phases_theoretical_reference.nxs', OutputWorkspace='correlation_spectrum') PoldiPeakSearch(InputWorkspace='correlation_spectrum', MinimumPeakSeparation=8, MaximumPeakNumber=30, MinimumPeakHeight=180, OutputWorkspace='peaks') DeleteTableRows(TableWorkspace='peaks', Rows='9,10') PoldiFitPeaks1D(InputWorkspace='correlation_spectrum', FwhmMultiples=4, AllowedOverlap=0.1, PoldiPeakTable='peaks') `
comment:30 Changed 6 years ago by Federico M Pouzols
- Status changed from verify to verifying
- Tester set to Federico M Pouzols
comment:31 Changed 6 years ago by Federico M Pouzols
Checking this and it's sibling systemtests/ pull request: https://github.com/mantidproject/systemtests/pull/10
comment:32 Changed 6 years ago by Michael Wedel
Refs #10263. Removing artifacts from merge conflicts resolution.
Changeset: 5757560d2b75a40d234f54533376875c5d9bf9a6
comment:33 Changed 6 years ago by Michael Wedel
Refs #10263. Added documentation for PoldiFitPeaks1D-v2
Changeset: 047b93c109a21f91a0136e4e446f064ed2448b66
comment:34 Changed 6 years ago by Michael Wedel
I realized there are some extra files from resolving a merge conflict, I will remove them. Also, the documentation was not updated properly, which I also fixed (good thing you just raised that point in the dev workshop :-) ). Could you update your local branch once the build has passed please?
comment:35 Changed 6 years ago by Michael Wedel
retest this please
comment:36 Changed 6 years ago by Michael Wedel
retest this please
comment:37 Changed 6 years ago by Michael Wedel
retest this please
comment:38 Changed 6 years ago by Federico M Pouzols
Aha, everything seems fine and the system tests had passed with the branch before today's updates. I'm passing them again just in case.
comment:39 Changed 6 years ago by Federico M Pouzols
- Status changed from verifying to closed
Merge pull request #10 from mantidproject/feature/10263_poldi_fit_close_peaks_combined
Feature/10263 poldi fit close peaks combined
Full changeset: 182cd552298809012ba7ab87a606b2794fdd4b11
comment:40 Changed 6 years ago by Federico M Pouzols
All is good, all tests pass, the example given above seems to work as expected, and it comes with good developer doc and user doc explaining the why and hows of the new algorithm version.
comment:41 Changed 6 years ago by Federico M Pouzols
Merge pull request #125 from mantidproject/feature/10263_poldi_refine_close_peaks_combined
Feature/10263 poldi refine close peaks combined
Full changeset: cd20b899f81fca1cbef8ee772420688668eef74d
comment:42 Changed 6 years ago by Federico M Pouzols
Merge pull request #125 from mantidproject/feature/10263_poldi_refine_close_peaks_combined
Feature/10263 poldi refine close peaks combined
Full changeset: cd20b899f81fca1cbef8ee772420688668eef74d
comment:43 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 11105