Ticket #7228 (closed: fixed)
ConvertToDiffractionMDWorkspace v2 performance
| Reported by: | Russell Taylor | Owned by: | Alex Buts |
|---|---|---|---|
| Priority: | major | Milestone: | Release 3.0 |
| Component: | Diffraction | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Tester: | Russell Taylor |
Description (last modified by Alex Buts) (diff)
The new version of ConvertToDiffractionMDWorkspace created under #6856 has been shown to be substantially slower by a couple of system tests (SXDAnalysis & TOPAZPeakFinding). Plots of the timing changes are attached. The performance of SXDAnalysis was recovered by switching it back to using version 1 of ConvertToDiffractionMDWorkspace, but obviously this is not a solution to the fundamental problem.
Investigate and improve the performance where possible.
Attachments
Change History
Changed 7 years ago by Russell Taylor
- Attachment SystemTests.SXDAnalysis.SXDAnalysis.runtime.v.revision.png added
Changed 7 years ago by Russell Taylor
comment:2 Changed 7 years ago by Nick Draper
- Milestone changed from Release 2.6 to Backlog
Moved to backlog at the code freeze for R2.6
comment:3 Changed 7 years ago by Alex Buts
- Status changed from new to inprogress
- Description modified (diff)
- Milestone changed from Backlog to Release 3.0
comment:4 Changed 7 years ago by Alex Buts
refs #7228 Initial rumbling around
Changeset: 95328a4821f867378ae0c6b64aa487cd5ef00071
comment:5 Changed 7 years ago by Alex Buts
refs #7228 different multithreading options exploration
Changeset: 7a00629055d6b03252b30babe04d986560c60e49
comment:6 Changed 7 years ago by Alex Buts
refs #7228 Ugly working multithreadede solution
Does not produce any significant acceleration as main performance drug is memory allocation, but otherwise working and bashing threads.
Changeset: bd502b291af1d67887caeb3e006af5533bb6bc11
comment:7 Changed 7 years ago by Alex Buts
refs #7228 removing threading stuff
Changeset: 585ae381054d083e0ec0853ad0a7509bb69fee30
comment:8 Changed 7 years ago by Alex Buts
refs #7228 enabled "Drop 0" option in convertToMD
Changeset: d7a381c9b86e6dc1fc374c37ddbbd7305db47e8f
comment:9 Changed 7 years ago by Alex Buts
refs #7228 Modified to reflect the changes to performance analysis
Changeset: 98d1e8c529799f6dc257659ac776fa91bc848361
comment:10 Changed 7 years ago by Alex Buts
refs #7228 fixed error in unit tests
Changeset: 64d6018c57740bbaecd3a34276ffd2e3695fbfaf
comment:11 Changed 7 years ago by Alex Buts
refs #7228 fixed strange error with kernel header
Changeset: beb8aee7ea94624212ec1cf5ff6ccbb93a6b672e
comment:12 Changed 7 years ago by Alex Buts
refs #7228 fixing unit tests
Changeset: 48afec1d0bd18cb138e9f726c4c9d731a5237eb5
comment:13 Changed 7 years ago by Alex Buts
refs #7228 Changed default "SingleEventPerBin" to true
to sustain existing unit tests.
Changeset: f3ee0f0c12dc8f1bbea95fa5fb0150ea2902b31f
comment:14 Changed 7 years ago by Alex Buts
refs #7228 System tests: correct options in peak finding
Changeset: 3589e44f24e92ba5854ac371bf2bc1c677f68395
comment:15 Changed 7 years ago by Alex Buts
- Status changed from inprogress to verify
- Resolution set to fixed
The main reason in such difference in the performance was different treatment of "0" by convertToMD (ConvertToDiffractionMDWorkspace V2) against ConvertToDiffractionMDWorkspace V1.
It is not entirely clear how to analyse these results, but I interpret the changes in the system tests at url:
as the substantial performance improvement due to the ticket's changes. It corresponds to the improvements I am seeing using local profiler.
Further improvement is possible but relates to thread scheduler and MDWorkspace basic operations so should be addressed by another tickets.
Tester should look at the performance changes according to the link supplied and may or may not test his own examples (it is not trivial as for some tasks performance changes are not obvious) and approve the ticket if he agrees that these changes improve performance.
The ticket code is both in Mantid and SystemTests branches -- both should be merged to master.
comment:16 Changed 7 years ago by Russell Taylor
- Status changed from verify to verifying
- Tester set to Russell Taylor
comment:17 Changed 7 years ago by Russell Taylor
Re #7228. Small tidy-ups (formatting, spelling).
Changeset: d4b8f200d327bc47e24d6e0f2516862a60986bd0
comment:18 Changed 7 years ago by Russell Taylor
- Status changed from verifying to closed
Merge remote branch 'origin/feature/7228_ConverToMDPerformance'
comment:19 Changed 7 years ago by Russell Taylor
Merge remote branch 'origin/feature/7228_ConverToMDPerformance'
comment:20 Changed 7 years ago by Russell Taylor
Both the system tests highlighted above have recovered their previous performance (though the TOPAZ one did so some time ago so perhaps that was something slightly different).
comment:22 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 8074
