Paring Down SCIRun
From NCRR Biomedical Software Development, Engineering, and Dissemination Wiki
The goal of this process is to reduce the size and complexity of the SCIRun infrastructure in order to make it more robust and flexible. There will be some loss of generality in the process, but this is a fair price to pay for the advanrages.
Remove dead code
- identify parts of the code that are not used
- find functions that have been replaced and should be retired
Remove or minimize dynamic compilation
- is essential for real time parsing of equations and commands in TransformData module
- what about leaving in that module but remove it from all other modules
- over time, replace the functionality we used in TransformData
- make scientific data calculator a student project sometime
- how will removal affect compilation time? hard to tell.
- todo: make a plan for how to remove this before they start.
- the cost will be in the range of types we can handle, and overhead in type checking
- how can we limit the data types we want to support and make a version of SCIRun
- we should remove unnecessary field classes like ITKImageField and ITKLatVolField
- we should minimize the number of mesh types we need
- virtual functions will require a more uniform interface for Field/Mesh classes
Move from configure/make to cmake
- cmake buys us a lot of portability because it works with local tools to replace configure/make operation
- medium to large size project
- would get us into the open software club
- logical players to work on this are Dav and Josh
- works with native tools to perform configure steps and make the program
Reduce third-party library dependencies
- this project already underway, e.g., we are getting rid of Xerces and Xalan
- Image Magik and mpeg are also things we can likely do without
- It would be great to maintain PNG support, as PPM does not export well to any tool most people are familiar with (--Jeroen)
- we are behind on ITcl and it remains a problem that we have to change it to work with SCIRun threads
- continue to check all third party libraries to see if we can remove dependencies
Provide improved development environment
- documentation is key, including documented sample code, tutorial for module creation, type names and descriptions
- how can we connect to threads at debug time and track bugs?
- could we organize the code into more libraries that people link to so that changes in a single header file, for example, require complete new builds from all developers?
- are there better tools? Tau, SGI has released open source version of CaseVision
- some way to slow down and query a dataflow network? Or generate a log of all steps?
- Do we have a set of symantic rules about data flow so that people have a better idea of what the system is doing and what we can expect it to do? Formal set of rules would help (documentation)
Wrapping of ITK functionality
- not used a lot and some trouble to maintain
- Darby will remove the wrapping
Memory management
- still using Steve's malloc code
- goal of that is to manage allocations in a way that reduces fragmentation
- this gets messy when there are different systems that link together
- we should at least catch allocation errors and report them to user in a clear way
Module/Algorithm separation
- if we are redoing dynamic compilation, we just as well move the field algorithms out of the modules and into proper classes
- this would remove all dynamically compiling code from GUI and into the Core libraries
- allows for more flexible GUI building and cleaner separation of the code
- makes code easier to reuse
Jeroen's List
Listed by priority:
Harmonize Mesh classes
We should harmonize the Mesh classes. Functions like get_center should use the basis functions, volume and interpolation schemes as well. Furthermore a lot of meshes are missing essential functions due to which dynamic compilation often fails, often calls with certain iterators are not handled. For instance TriSurfMesh is missing quite some functions, which is hindering progress in the collaborations. Completing them would solve already some problems. Then there are functions that were needed for the FEM, these can go and should be removed from TetVolMesh, HexVolMesh and TriSurfMesh. Likewise there are functions implemented in some mesh classes and not in others, we should try to remove these as much as possible.
Update: We identified a series of functions that are missing and Micheal will be working on it.
Bug in Viewer
We should track down the synchronizarion errors in ViewSlices and Viewer. I was demonstrating Alex (one of Rob's students) SCIRun and we had really frequent crashes. Basically SCIRun gets into a situation where several threads are waiting for each other and the whole program hangs or the second situation is that I get a SIGBUS in the viewer/openGL thread. The errors occur more frequently when I have a different process running the background doing a lot of work. This will influence the multi threading and basically the order in which threads are served. Since this leads to problems my suspicion goes to a not-thread safe implementation of some data. This makes using BioImage really hard. I have been hearing and experiencing a lot of problems with this. Not only is the current version affected, 1.24.2 is suffering from this problem as well. Since a large focus is on visualization we need to fix this problem, as we need to be able to use SCIRun while we restructuring SCIRun, the last real stable version was 1.24.1 which is a really old one. Anyway in restructuring SCIRun, this problem needs to be solved anyway.
Update: The developers are aware of this problem and are looking into its origin. All clues indeed seem to suggest a thread safety issue. McKay will try to track down this problem.
Algorithm/Module separation
Separating out algorithms from the the modules. We need this anyway when doing a clean up it will help with the dynamic compilation. So everything including the dynamic compilation part should go in a class where I can make a simple call to it just supplying handles to the Field/Matrix objects. The Algorithms should go to core/algorithms. This will mostly separate Core and Dataflow properly so people can use it for different interfaces. These libraries should be able to be initialized with the ProgressReporter so information to the user can go anywhere. We need not only exceptions to be thrown but as well warnings and remarks being forwarded, likewise progress for progress bars.
Update: Drafted a quick outline on this:
For the Algorithm/Module separation, I'd like it to be defined like this:
I would like to have a template class that takes a ProgressReporter pointer (or Module pointer) and has simple inline functions build in to forward errors. For example:
class AlgoLibrary {
public:
AlgoLibrary(ProgressReporter* pr=0);
private: ProgressReporter *pr_;
inline void error(std::string error); // forward error to ProgressReporter inline void warning(std::string error); // forward error to ProgressReporter inline void remark(std::string error); // forward error to ProgressReporter inline void compillationfailure(std::string error); // forward error to ProgressReporter
}
Then derived from that I would like to see a algorithmic library that is derived from this base class with field algorithms implemented as functions:
class FieldAlgo : public AlgoLibrary {
public: // constructor
FieldAlgo(ProgressReporter* pr=0) :
AlgoLibrary(pr)
{ };
// examples of function calls
bool Unstructure(FieldHandle& input, FieldHandle& output); bool MergeFields(std::vector<FieldHandle>,FieldHandle& output, double tolerance); bool Transform(std::vector<FieldHandle>, FieldHandle& output, std::string function); ..... etc ....
// Functions return false if an error ocured and the error is forwarded to the ProgressReporter
// For regression testing // The idea is to be able to build small example meshes of each type dynamically for testing
bool BuildExampleMesh(FieldHandle& output, std::string meshclass, std::string basistype); bool BuildExampleField(FieldHandle& output, std::string meshclass, std::string basistype, std::string datatype);
bool CompareMeshes(FieldHandle& input1, FieldHandle& input2, bool& isequal) bool CompareFields(FieldHandle& input1, FieldHandle& input2, bool& isequal)
}
Similarly in the end it would be great to have a class called MatrixAlgo, DataIOAlgo, etc ....
All these classes should go into Core/Algorithms
The idea is to include simple functions for building example meshes. We can then loop over these functions and build example meshes and then feed them to each of the functions in the mesh. This way we could build a simple regression testing procedure as a stand alone program. When running the program it will try to dynamically compile algorithms for each mesh type, hereby testing whether everything compiles dynamically without the need of having to start the whole TCL infrastructure!
Each function in the Algorithm class should assume it can receive any type of field class and it should report an error if it is not appropriate, by feeding it a range of example meshes we can effectively test whether these functions except every possible field. We could as well test the dynamic compilation more automatically, as the test program can be very simple program that does not need the SCIRun interface, but just uses FieldAlgo library to generate simple Example meshes, this way we could test whether dynamic code works and whether functions still result in the same mesh.
Reorganizing code and removing dead code
All objects that derive from DataType should go into Core/Datatypes, hence as well ColorMap and ColorMap2, when that is done Bundle can go into Core/Datatypes as well, making the code more organized. This will solve circular dependencies.
Update: Micheal is working on this
We could remove Core/Process , this code is unfinished and my code in SystemCall is more or less doing the same, Perhaps we should rename SystemCall to Process and then it is virtually the same but with a better name.
Update: ?
Some dead classes we should remove (old projects of mine) Core/ICom/sshagent (I am not using this), Core/Dataypes/NrrdString (concept has completely been removed), Core/Datatypes/NrrdScalar (I am not using concept anymore).
Update: These have been removed.
Update Insight package
We should remove ITKImageField and ITKLatVolField and replace them with GenericField. Only the container class in GenericField should be overloaded with an image the rest is just dead weight, making SCIRun too complicated.
Update: not yet working on this.
We should make the fdata member in GenericField a handle to the container. This way fields can share data with other objects and it would prepare the system for memory blocks to be shared across objects. It would as well allow for the special ITK classes to be removed, as we could turn the Image class as well into a container class and make the sharing of data between ITK and SCIRun way easier.
Update: We need to discuss this with the other SCIRun groups....
Both Josh and I feel that the ITK Package needs to be updated. We are going to sit down in January and figure out what needs to be done here. The issues above will be included in this process.
File Converrters
We should move all the file converters for NCRR center into SCIRun itself, we should not have too many stand alone programs that will only give the illusion that we are writing an operating system. If we could do this we could simplify code and move the converters into the Core of SCIRun. This will make it easier to find the proper pieces of the code.
Update: Micheal is willing to give this a try.
Graphical output
We should think of removing ImageMagick and replace it by libPNG. Image Magick can become an optional package. If libPNG is there then the default output of the viewer should be .png and .ppm. Almost every graphics program can support png and it is a completely open format. So that would improve usability.
Update: We will make png default output format in viewer and resolve the question of ImageMagick vs. LibPNG later, when we have the CMake system in place.
Further integration of Interpolation schemes
We should enforce proper interpolation schemes. Currently a lot of code for hexahedrals is just averaging the values on the nodes, as soon as the element is not cubic it is not proper. At least depending on how we define our interpolation scheme. For simplicity we should do this conform the basis functions. Hence functions like get_center and modules like ChangeFieldBasis should use the interpolation schemes. This makes SCIRun more consistent, which in the will simplify documentation.
Update: We still need to work out details on where we need to implement this.
Restructuring BioPSE
In BioPSE some modules use units as properties and scale things accordingly. However only a fraction of the modules supports this and hence this can lead to unpleasant surprises. In some case you work with everything in cm and put units cm and the FEM module rescales the matrix to meters and in other cases you expect it to rescale and it does not. I would suggest removing this feature completely and just let the user enter the data in the proper units when he or she is loading the data. For instance in the Duke projects we do computation all in micrometers or centimeters and rescaling if I define a unit will make things only worse. Hence I would remove this functionality.
Update: We agreed that modules should not do any conversions, but should warn when units do not match
The other problem is that BioPSE is not widely used and hence the center is not visible (BioPSE and the center are often directly linked, I know we do way more, but it is not always visible). Hence the idea to revitalize the BioPSE package.
Some thoughts on this:
- BioPSE should contain core functionality for our center. It could for instance contain the Insight basic datatypes, the core of the matlab libraries, most file converters etc, I might move the String data types over to the BioPSE core.
- Depending on the BioPSE core we have a couple of packages e.g. BioPSE_ImageProcessing, BioPSE_MeshGeneration, BioPSE_ModelGeneration, BioPSE_Simulation. Common components go in the core of the BioPSE package and hence for anyone to use any of the NCRR functionality will need to install the BioPSE package. This will help determine what the NCRR center is developing. Perhaps we should call packages like BioPSE_Matlab etc, to specifically spell out what the NCRR domain is.
- This could make BioPSE the extension of the SCIRun Core we rely on in our developments. Hence the name SCIRun/BioPSE would get more meaning.
- After we are done with improvements in the Core, we could thence better mark our development field
Update: We at least agreed to think about how to organize our packages and we probably have a brainstrom session in January.
Pio and Dynamic compilation
One discussion point: Currently a lot of the higher order classes are not instantiated for the Pio system. If we are going to support these we might want to implement the suggested model for the dynamic compilation, predefine most of them and when a class is not found use dynamic compilation to build it. Though here we have the opposite problem as with the dynamic compilation. The amount of possible field classes is so large that the list of instantiations need be really large. Or we complete the list in cd_templates or we do this with dynamic compilation. Another problem is that looking up the symbols in the dynamic libraries takes a lot time. Some networks use most execute time to find dynamic libraries and dynamically compiled files.
Update: I drafted some ideas on how to deal with the dynamic compilation:
Phase-I
I think the first step in simplifying this should be to combine the concepts of Pio and DynamicCompilation. Actually it is not simplifying it is just generalizing concepts we already have developed and have experience with and the combine them
In Pio we have a list of all available makers and then choose the maker we need and in DynamicCompilation we have a function that dynamically creates a piece of code and then loads the maker.
In my opinion the best way forward in phase-1 is to combine both concepts:
- Create a list of algo_makers, similar to what we currently have for the field_types needed for Pio.
- Add static constructors to all algorithms we have for dynamic compilation and make them register themselves in the list of available makers
- In loading dynamic code : we first search the list of makers constructed from the static constructors (if it is there the algorithm is already available in the Core),
then search on-the-fly libs with the generated filename, if nothing is found generate code that will do the job. This would improve performance as the Pio way of static constructors that copy the algorithm maker on a list will be faster than looking up symbols in a dynamically loaded file.
- Add precompiled instantiators for often used version, e.g. like the cd_field_template files in Core/Datatypes for commonly used mesh types and data types (double, Vector, Tensor)
- Make Pio use the same order (hence Pio then could dynamically compile if needed).
This way we could have multiple distributions with the same source tree: For the windows distribution we add a compilation of all algorithms we think we need (we can do this by setting options in the makefile), for a developers version we skip over all of those and let Pio and Algorithms do their dynamic compilation (the system would be smart enough if it does not see a statically registered maker, then it needs to build the class), and combinations of both strategies depending on how much we want to be prebuild. I can imagine that I want to have a lean version (everything dynamically compiled) so I save memory.
The cleanup in this case would come from the merger between Pio and Dynamic compilation. It would fix the problem we currently have that we do not have Pio support for all the higher order stuff.
Having algorithms in a cd_template_fields construction will reduce the amount of files we have and as well some compiler overhead (not every individual algorithm needs to be linked against all libraries, looking at on-the-fly-libs that is currently a problem) and probaly will speed up executing of networks.
Phase-II
- Add an interpreter for dynamically compiled functions. We theoretically could keep a dynamic version (highly optimized for speed) and an interpreter version. If dynamic compilation is not available we should do it with the interpreter.
- At the same time we might integrate the functionality I already built. I have functions like this that are more flexible. I already have modules that replace TransformData completely and are more flexible and have already a Algorithm/Module separation (not fully separated, but close to what I envision). My dynamically compiled system adds a lot of functions directly to the interface, functions like computing fractional_anisotropy, etc are already defined, similarly the framework I have allows for mixing data from Matrices and Fields, which I will need for scripting SCIRun (A project I like to work on next year). I don't have an interpreter function yet, but I'd like to add it the framework I already setup a while ago. Combining a newly to develop interpreter and the framework I already have and then adding it to the SCIRun-Core may be the way to go.
Phase-III
- We need to figure out what to do with algorithms that lead to too many combinations. For this we would need virtual functions.
- Add virtual functions to SCIRun. Again we could have a speedy version using dynamic compilation and one that uses virtual functions.
- We could see what the performance hit is and choose accordingly. For projects where windows compatibility is paramount we only implement functions virtually , where speed is paramount we do it through dynamic compilation, if we need both we implement both versions.
- If the number of possible algorithms is less then lets say 50 or something we just precompile all of them for windows and solve it like that. If we get in a lot of combinations we need to rewrite the algorithm to use virtual functions or split it up in smaller components (sometimes an alternative for virtual function calls). This will reduce the amount of algorithms we need to rewrite.
-- Jeroen
