Opened 13 years ago

Closed 13 years ago

#102 closed defect (fixed)

Improve Duchamp's management of vector lists

Reported by: MatthewWhiting Owned by: MatthewWhiting
Priority: normal Milestone: Release-2.0
Component: Code base Version: 1.1.10
Severity: normal Keywords:
Cc:

Description

The ASKAP ticket #3133 (http://pm.atnf.csiro.au/askap/issues/show/3133) details some issues with the efficiency of the code that can do with attention. It appears to be relatively simple fixes that are required. Should speed up things as well.

Attachments (1)

merging-issues.txt (2.2 KB) - added by MatthewWhiting 13 years ago.
Description of how merging problem was solved.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 13 years ago by MatthewWhiting

Status: newassigned

As a benchmark, I ran the ASKAPsimSL3 test with threhsold=0.005 on cuscus (with Duchamp 1.1.10) and it took 10:17:16 to run. Running with 0.02 (a more reasonable threshold - the 0.005 one just finds the entire field :) took 5:42 (ie.342 sec).

comment:2 Changed 13 years ago by MatthewWhiting

I have committed a bunch of changes in [770]. These cover:

  • Changing the use of erase() in removing merged detections to doing the copying in-place. We make use of a vector<bool> to keep track of which detections are valid.
  • This led to looking at the optimisation of other functions. There were a couple that used erase() as well, but the big gains were looking at the areClose tests. New functions have been written that reside in the classes to take care of the tests, and removing their use of Scans has sped things up quite a bit.

The second benchmark listed above (threshold=0.02) now comes out to 2:54 - ie half the time. And most of that is in the parameter calculations.

Something to check however, is whether we get all the pixels appropriately - using Tara's test from last milestone shows that we get fewer sources. This seems to be because some have merged together with the new code.

Changed 13 years ago by MatthewWhiting

Attachment: merging-issues.txt added

Description of how merging problem was solved.

comment:3 Changed 13 years ago by MatthewWhiting

Spent a day chasing up the differences in the results for Tara's test. Tracked it down to a miscalculation of the length of a scan in the Object2D::isClose() function. Fixing that results in the same results, but in >30s quicker time (1:10 cf 1:47 for v1.1.10). Details of the investigation in attached file.

comment:4 Changed 13 years ago by MatthewWhiting

Oh, the necessary change was in [771].

comment:5 Changed 13 years ago by MatthewWhiting

A few more tweaks made in [776] in particular, where are couple of the tests were wrong in their logic.

Not sure I completely understand the speed issues, as I was getting 67 sec at one point for the Tara test, but then made a couple of changes that should not have affected things, and it blew out to 80 sec. (Running with time and with Shark profiling gives the same results - things just seemed to be running slower. Not clear that it's due to anything else running at the same time ...)

comment:6 Changed 13 years ago by MatthewWhiting

NB - just tried Shark on that test, and got 67 sec, so it might have been a glitch with the build before...

comment:7 Changed 13 years ago by MatthewWhiting

Resolution: fixed
Status: assignedclosed

Updating this - after all the changes (to [781]), the ASKAPsimSL3 benchmark with threshold=0.02 runs in <23sec!

Pretty happy with this, and will close the ticket. If there are any further issues following integration into askapsoft, then re-open...

Note: See TracTickets for help on using tickets.