Ticket #5675 (closed: fixed)

Opened 8 years ago

Last modified 5 years ago

Make Show Detectors multi-threaded

Reported by: Stuart Campbell Owned by: Federico M Pouzols
Priority: major Milestone: Release 3.3
Component: MantidPlot Keywords: NewStarter
Cc: Blocked By:
Blocking: Tester: Andrei Savici

Description (last modified by Nick Draper) (diff)

If you right click on a workspace and select "Show Detectors" then this can take a while (around 20-30 seconds for HYSPEC).

Looking at 'top' it looks like this calculation is single threaded.

Change History

comment:1 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.3 to Release 2.4

Moved to milestone 2.4

comment:2 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.4 to Release 2.5

Moved at the code freeze for release 2.4

comment:3 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 2.5 to Release 2.6

Moved to r2.6 at the end of r2.5

comment:4 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:5 Changed 7 years ago by Nick Draper

  • Status changed from new to assigned

Bulk move to assigned at the introduction of the triage step

comment:6 Changed 6 years ago by Nick Draper

  • Keywords NewStarter added
  • Description modified (diff)

comment:7 Changed 6 years ago by Nick Draper

  • Owner changed from Anyone to Federico M Pouzols
  • Milestone changed from Backlog to Release 3.3

comment:8 Changed 6 years ago by Federico Montesino Pouzols

  • Status changed from assigned to inprogress

"Show detectors" parallelized with an OpenMP loop, re #5675

Changeset: 3638d676b607256db0a5a98bc9d74b9d0d3a5910

comment:9 Changed 6 years ago by Federico M Pouzols

The show detectors loop is now parallel and working fine. But it is running too fast on my computer (around 1/2 a second with this HYSPEC dataset, or WISH, as available in the systemtests repo). Run times are around 0.5-0.55 s. Looking for a data file with many more detectors, to see if we can confirm any significant speedup...

comment:10 Changed 6 years ago by Federico M Pouzols

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

Now I tried with some MERLIN data files. These have 69632 detectors which is enough to start noticing a bit of speedup.

The run times (just 5 repetitions) are in the following ranges (s):

  • Without parallel loop: 2.84-2.99
  • With parallel loop: 1.90-1.96

So I'll take this as sufficient evidence that it is not only working but also running faster for large (many detectors) datasets.

This has been run on a debian virtual machine that uses half of my i7 processor. Not as great results as it can sometimes be with OpenMP but still a decent speedup.

If you'd like to test this with similar data, from an ISIS windows workstation you can get MERLIN data here:
Isis\inst$\NDXPOLARIS\Instrument\data\

Last edited 6 years ago by Federico M Pouzols (previous) (diff)

comment:11 Changed 6 years ago by Andrei Savici

  • Status changed from verify to verifying
  • Tester set to Andrei Savici

comment:12 Changed 6 years ago by Federico M Pouzols

  • Status changed from verifying to reopened
  • Resolution fixed deleted

comment:13 Changed 6 years ago by Federico M Pouzols

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

comment:14 Changed 6 years ago by Andrei Savici

  • Status changed from verify to verifying

comment:15 Changed 6 years ago by Andrei Savici

  • Status changed from verifying to reopened
  • Resolution fixed deleted

I am using Ubuntu 14.04. I loaded an empty instrument (TOPAZ), then tried show detectors. It is extremely slow (I stopped it after 40 minutes), because it keeps outputting "QPixmap: It is not safe to use pixmaps outside the GUI thread" message for every pixel (millions)

comment:16 Changed 6 years ago by Federico Montesino Pouzols

  • Status changed from reopened to inprogress

Don't call QtGUI methods (setText) outside the GUI thread, Re #5675

Changeset: 206c283a09fb16789d4ce816117bc58da775c76e

comment:17 Changed 6 years ago by Federico M Pouzols

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

Oops, my bad. I had left the setText() calls inside the multithreaded loop, and those cannot be safely called outside of the normal Qt GUI thread. Somehow I hadn't got those warning messages.

Now it should be working safely, at the cost of a bit of overhead. Actually, it seems that that overhead has been compensated by a few other minor changes, and it even looks like splitting the loop into two had a positive effect. The new run times for the same 69632 detectors are:

  • without parallel loop: 2.57-2.60
  • with parallel loop: 1.35-1.38

comment:18 Changed 6 years ago by Andrei Savici

  • Status changed from verify to verifying

comment:19 Changed 6 years ago by Andrei Savici

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/5675_multithreaded_show_detectors'

Full changeset: 29660a278d285a7a234f3c7fdb48088e27f8c7ba

comment:20 Changed 6 years ago by Andrei Savici

A similar speed up was observed on TOPAZ (from 40 seconds to 20)

comment:21 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 6521

Note: See TracTickets for help on using tickets.