Error messages from pipelines indicate algorithms by their ptr address and class name. With complex pipelines this is usually insufficient to identify the algorithm that sends the message. It would be very helpful if algorithms could carry a descriptive name.
I propose to introduce a vtkAlgorithm char* ivar Name and a GetDescriptionForMessage() method that returns the description that is used by vtkExecutive in error and debug messages.
The error-messaging code in VTK is at the level of
vtkObject, so it would be easier to put changes there, instead of in the algorithm classes. Something like a
SetObjectName(const char*) method for
vtkObject. If not NULL, the object name could be included in all error logging.
Would a call stack not be more useful? Are you wanting this feature to work in a release build or a build containing debug info?
A call stack would be very useful, but a printed stack is too verbose I think. I can get a call stack by throwing an exception in my vtkOutputWindow when an error event is raised and then I can navigate it in the debugger. The object name is useful in more ways though: we have a debugging tool that generates a graph of our pipelines, this would use the name too. Currently I store the name in the vtkInformation of the vtkAlgorithm, having it as an ivar in vtkObject allows using it in messages too.
Since you know the names associated with each algorithm at design time could you not just create a std::map as part of the bootstrapping for this purpose and pass it to your debugging tool?
Instead of adding just one more string member, it would allow lots of simplification in application code if vtkObject could store custom properties (simple std::string->std::string map) - the same way as QObject has dynamic properties and similarly how you can add custom data to any Python object anytime. The only reason I would not advocate this very strongly is that it could be too tempting to overuse it (instead of creating a new subclass that has additional members to store extra metadata, developers would be tempted to just store the extra information in properties).
The object name would be helpful for vtk error and warning messages where now you only get a class name and a pointer. The messages appear in output logs where the pointers do not really mean anything but an object name would be very helpful.
Although not on the level of vtkObject this is already possible in vtkAlgorithm and vtkDataObject, which have a vtkInformation in which you can store data. For example, I already store the algorithm name in the vtkInformation of vtkAlgorithm instances. The error messaging however does not allow me to print this name.
I agree that adding a “name” metadata and printing that in logging macros would be useful.
I just raised the need for general-purpose properties because if we add a string variable that developers can use to store some kind of application-specific object name then it is inevitable that it will be used for more than just logging: that string will be (mis)used in applications to store other metadata, such as an ID or other properties.
So far VTK was very strict in not letting storing any application-specific custom properties in vtkObject (you could only sneak in custom properties at specific subclasses as vtkInformation or FIELD data). If we introduce an application-specific custom “name” string then we should consider making it more structured. Qt’s well-proven system of object name (string) and properties (map from string to string or vtkVariant) would work well.
That said, adding an object name could be a first step towards letting more application-specific metadata storage in vtkObject. So, I support adding just a name ivar to vtkObject now and display that in logs.
Surely if the logger has access to each vtkObject instance then it could lookup any extra text information in a map?
True, but for the error/warning macros to use such a map would require more code (exposing more headers through the vtkGetSet.h) to pass the name to the stream and it would rely on a ‘predefined’ keyword “objectname”.
I would do the same as Qt: introduce a
name member variable (not a dynamic property, just a simple string) and add dynamic property system (map from string to vtkVariant). They are related because if we add a member variable that is meant to store an application-specific string then developers will not be able to resist the temptation and will store various properties in it. Adding properties will allow to keep the
name clear (and many simplifications in application code, as in many situations we can avoid maintaining external maps).
I store names of instances of algorithms in their information. Simple create a new string key and name it say “algorithmName”. You can then append to the information already available in all algorithms. This is quite general and I cannot see why we need another mechanism. I may be wrong
Yes, it could work for filters, but the VTK logging macros are defined for vtkObject, so it would be more appropriate to handle this at vtkObject level.
It seems to me that the problem needs to be addressed in the logging macros then.
I currently use the approach that @Jens_Munk_Hansen described too. An alternative solution without having to interfere too much in vtkObject could be to add a virtual function
vtkStdString vtkObject::GetObjectDescription() const;
which returns a string that can be passed into the output stream. The vtkObject implementation would return the object’s class name and pointer, like what is now output through the macros. The vtkAlgorithm class could have an algorithm_name string key to store the algorithm name in its vtkInformation and the GetObjectDescription could be overridden to return it if set. A disadvantage is that vtkStdString will be exposed through vtkObject.h.
Adding a virtual function to
vtkObjectBase to get a name or description could be more powerful than just a simple std::string member, as it would give more flexibility. For example, it would allow using existing application-specific identifiers that are added in derived classes. (In the VTK-based MRML library, each MRML node has an ID and name and this way we could include them in the log messages by simply overriding the GetObjectDescription method). It would be also similar to how class name and detailed description of the class is retrieved (using
PrintSelf virtual methods).
vtkObjectBase::PrintHeader would need to be updated, too, to use
GetObjectDescription instead (or in addition to)
A std::string return value should not be a problem. It is used in hundreds of VTK classes. As far as I know vtkStdString is deprecated (it can be replaced by std::string).
Can this not be achieved by overloading the << operator in vtkAlgorithm to output vtkInformation key/values in the warning/error macros?