MantisBT - ParaView
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0013834ParaView(No Category)public2013-01-29 19:432016-08-12 09:59
ReporterAlan Scott 
Assigned ToKitware Robot 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusclosedResolutionmoved 
PlatformOSOS Version
Product Versiongit-master 
Target VersionFixed in Version 
ProjectSandia
Topic Name
Typeincorrect functionality
Summary0013834: ProgressHandler needs fixing, error checking, consistency and COMMENTS.
DescriptionThe progress handler needs to have it's throttle fixe. File vtkPVProgressHandler.cxx, member function vtkPVProgressHandler::ReportProgress(). Progress should NEVER be outside of the range of 0 to 1, so don't send MPI calls for out of range progress. Also, make sure to throttle progress for 0.0, since many functions in the code incorrectly send this billions of times.

I changed
  if (progress <= 0.0 || progress >= 1.0 ||
    this->Internals->ProgressTimer->GetElapsedTime() > this->ProgressFrequency)
to
  if (progress >= 0.0 && progress <= 1.0 &&
    this->Internals->ProgressTimer->GetElapsedTime() > this->ProgressFrequency)


The Progress Report code needs a way to find error cases with progress. I suggest adding the following at the top of the vtkPVProgressHandler::ReportProgress() function, which I assume only is printed for debug builds:

  if (progress < 0.0 || progress > 1.0 )
    vtkWarningMacro(<< "vtkPVProgressHandler::ReportProgress - "<<
        " progress out of range [0.0-1.0] " << progress);



The Progress Handler code needs to have progress be consistent throughout. In most functions, progress goes from 0.0 to 1.0. In the call to SetLocalProgress(), progress becomes a range of 0 to 100, and inside this code makes a call that requires it to be divided by 100! Keep 0.0 to 1.0 throughout. Be sure to change LastProgress = progress*100.0 (assuming it even gets used, which I couldn't find).



Last, please document the range for progress and change the variable for progress if it has a different range. It was confusing what it should be. (progress100?)
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
child of 0013831closed Kitware Robot Master bug - Progress Handler is a mess 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2013-01-29 19:43Alan ScottNew Issue
2013-01-29 21:23Alan ScottNote Added: 0030274
2013-01-29 21:24Alan ScottNote Edited: 0030274bug_revision_view_page.php?bugnote_id=30274#r555
2013-01-29 21:29Alan ScottNote Added: 0030275
2013-01-29 21:50Alan ScottRelationship addedchild of 0013831
2013-01-30 16:58Alan ScottNote Added: 0030283
2013-04-29 15:24Alan ScottNote Added: 0030629
2013-05-01 13:55Nikhil ShettyAssigned To => Nikhil Shetty
2013-06-05 15:59Nikhil ShettyAssigned ToNikhil Shetty =>
2013-06-05 16:00Nikhil ShettyAssigned To => Nikhil Shetty
2013-06-05 16:37Nikhil ShettyAssigned ToNikhil Shetty =>
2016-08-12 09:59Kitware RobotNote Added: 0038325
2016-08-12 09:59Kitware RobotStatusbacklog => closed
2016-08-12 09:59Kitware RobotResolutionopen => moved
2016-08-12 09:59Kitware RobotAssigned To => Kitware Robot

Notes
(0030274)
Alan Scott   
2013-01-29 21:23   
(edited on: 2013-01-29 21:24)
In vtkPVProgressHandler.cxx, where this->ProgressFrequency is set, please set it to something like 1 second. 2 is a bit long, and is probably just trying to mask errors.
(0030275)
Alan Scott   
2013-01-29 21:29   
Please add a print statement to the code, which is normally commented out, which will tell a developer if the ProgressHandler is working. For instance, in SendProgressToClient, below all of the if() tests, do a StopTime() then print out GetElapsedTime and progress. Then, test with multiple servers, multiple files. Make sure that progress increments as it should, and doesn't put too many lines out. If a developer doesn't want to page through the output, there are too many lines written out (and ReportProgess is sending MPI calls between processes).
(0030283)
Alan Scott   
2013-01-30 16:58   
As an idea, how about have this print statement start printing status if some environment variable, or CMake flag is on? That way we could easily test when wanted, but otherwise we wouldn't get flooded with output.
(0030629)
Alan Scott   
2013-04-29 15:24   
I believe this one has been fixed. It was fixed by removing the MPI code, and only using process 0 for the updates. This removed a hang and slowdown due to flooding the network with update data.
(0038325)
Kitware Robot   
2016-08-12 09:59   
Resolving issue as `moved`.

This issue tracker is no longer used. Further discussion of this issue may take place in the current ParaView Issues page linked in the banner at the top of this page.