View Issue Details

IDProjectCategoryView StatusLast Update
0004191Slicer4Module LandmarkRegistrationpublic2018-03-02 11:07
Reporterwinnubstj Assigned Topieper  
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
PlatformOSWindowsOS Version
Product VersionSlicer 4.5.0-1 
Target VersionFixed in VersionSlicer 4.6.0 
Summary0004191: Slow addition fiducial points in Landmark Registartion
Description

Adding new fiducial points becomes incredibly slow around the 100 point mark (0000020:0000030 seconds). Strangely enough adjusting these points and moving them around is smooth again after that. There is no difference if I switch from the thinplate to the affine transformation. Also occurs on my Ubuntu install.

Steps To Reproduce
  1. Start module with 2 volumes (1GB each).
  2. Add 100 points.
TagsNo tags attached.

Activities

pieper

pieper

2016-05-17 11:37

administrator   ~0013877

Just to confirm, can you try using some of the sample data (from the SampleData module) and confirm you see the same behavior as you do with your data? If so then it will be possible to easily reproduce and investigate the issues. We have the 'self test' feature in Slicer and it should be possible to create a scenario with 100 fiducials and then profile where the issue comes up.

winnubstj

winnubstj

2016-05-17 11:56

reporter   ~0013878

Thanks for the quick reply! I tried it on the sample data set 'Pre-PostDentalsurgery' but had the same thing (also now using nightly build). Slow down becomes apparent around 40 point but scales quickly.

pieper

pieper

2016-05-17 16:28

administrator   ~0013879

I have seen the slowdown with many points - I need to use a profiler to get an idea of where we might be able to speed things up.

Regarding the lack of an update when going from thin-plate to affine, you can trigger an update by selecting Similarity and back to Rigid. Can you check that?

winnubstj

winnubstj

2016-05-17 16:31

reporter   ~0013880

Yes I did notice the Similarity - Rigid update trigger. Sorry I intended to say that the slowdown is present in both the affine and thin plate registration mode

pieper

pieper

2016-05-20 10:25

administrator   ~0013885

Improvement implemented here:

https://github.com/Slicer/Slicer/pull/505

being tested.

pieper

pieper

2016-05-20 12:48

administrator   ~0013887

Committed as:

https://github.com/Slicer/Slicer/commit/fe45f62d27d16852fca3b8f28b18ebe51d260840

Unless something goes wrong, tomorrow's nightly build should include these changes and it would be great if you could test them.

pieper

pieper

2016-05-21 18:44

administrator   ~0013892

Unfortunately the nightly testing failed, so this will need to be backed out and we'll need to think if there's a better fix.

winnubstj

winnubstj

2016-06-03 14:56

reporter   ~0013944

Hello Steve,

I noticed yes that is really too bad! Is there currently any outlook for a fix for this behavior because we are still very interested in using the module.

Best,
Johan

pieper

pieper

2016-06-08 16:59

administrator   ~0013954

Found another thing to try - this is a localized change to landmark registration so there should be no side effects to the rest of Slicer.

Committed in r25172

Should be available for testing in tomorrow's nightly build if all goes well.

jcfr

jcfr

2016-06-08 23:57

administrator   ~0013956

Fix effectively integrated in http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=25173

winnubstj

winnubstj

2016-06-17 14:12

reporter   ~0013987

Last edited: 2016-06-20 20:17

View 2 revisions

Hello Steve,

Thanks for the update! Its working much better now. I still get a slowdown around 150 points where adding new points takes around 10 seconds. If a further speed up is not possible could I maybe try working on sets of 100 at a time and later add them together? Do you have some suggestions of how I could do this? Also I added a 'feature' issue to the tracker concerning the deletion of landmarks.

Thanks again,
Johan

Related Changesets

Import 2017-06-07 23:51:09: master fe45f62d

2016-05-20 12:44:02

pieper

Details Diff
PERF: 0004191 don't update widgets in batch mode

This really improves the performance of LandmarkRegistration,
particularly when there are large numbers of fiducials
being manipulated. I haven't seen any side effects.

ENH: request future update when ignoring batch mrml event

Per suggestion from Andras, include a request for future
update to be sure that the widgets are updated
before the render happens.

BUG: 0004191 update landmark registration

Addresses slowdown with large numbers of fiducials
by entering batch mode for bulk operations on the scene.

From: Steve Pieper <pieper@isomics.com>

git-svn-id: http://svn.slicer.org/Slicer4/trunk@25094 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Modules/Loadable/Markups/MRMLDM/vtkMRMLMarkupsFiducialDisplayableManager2D.cxx Diff File
mod - Modules/Loadable/Markups/MRMLDM/vtkMRMLMarkupsFiducialDisplayableManager3D.cxx Diff File
mod - SuperBuild.cmake Diff File

Import 2017-06-07 23:51:09: master 7a1d7300

2016-05-21 19:03:04

pieper

Details Diff
BUG: 0004191 back out optimization that broke tests

https://github.com/Slicer/Slicer/pull/505

From: Steve Pieper <pieper@isomics.com>

git-svn-id: http://svn.slicer.org/Slicer4/trunk@25106 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Modules/Loadable/Markups/MRMLDM/vtkMRMLMarkupsFiducialDisplayableManager2D.cxx Diff File
mod - Modules/Loadable/Markups/MRMLDM/vtkMRMLMarkupsFiducialDisplayableManager3D.cxx Diff File

Issue History

Date Modified Username Field Change
2016-05-17 11:14 winnubstj New Issue
2016-05-17 11:14 winnubstj Status new => assigned
2016-05-17 11:14 winnubstj Assigned To => pieper
2016-05-17 11:37 pieper Note Added: 0013877
2016-05-17 11:56 winnubstj Note Added: 0013878
2016-05-17 16:28 pieper Note Added: 0013879
2016-05-17 16:31 winnubstj Note Added: 0013880
2016-05-20 10:25 pieper Note Added: 0013885
2016-05-20 12:48 pieper Note Added: 0013887
2016-05-20 12:49 pieper Status assigned => feedback
2016-05-21 18:44 pieper Note Added: 0013892
2016-06-03 14:56 winnubstj Note Added: 0013944
2016-06-03 14:56 winnubstj Status feedback => assigned
2016-06-08 16:59 pieper Note Added: 0013954
2016-06-08 16:59 pieper Status assigned => feedback
2016-06-08 23:57 jcfr Note Added: 0013956
2016-06-08 23:57 jcfr Status feedback => resolved
2016-06-08 23:57 jcfr Fixed in Version => Slicer 4.6.0
2016-06-08 23:57 jcfr Resolution open => fixed
2016-06-17 14:12 winnubstj Note Added: 0013987
2016-06-20 20:17 jcfr Note Edited: 0013987 View Revisions
2017-06-10 08:51 pieper Changeset attached => Slicer master 7a1d7300
2017-06-10 08:51 pieper Changeset attached => Slicer master fe45f62d
2018-03-02 11:07 jcfr Status resolved => closed