Conv merge
Implemented optional convolution filter, fixed various segfaults due to dangling pointers
Merge request reports
Activity
mentioned in issue #12 (closed)
mentioned in issue #18 (closed)
Those didn't survive the refactoring of the convolver and convolution classes (which were almost totally rewritten), but I can add them back in if there is a desire. The convolver class and the peak finding dialog are implemented in such a way that it is really easy to add new kernels. For biodiff the most important filter to get working was the annular one because it is what they are already using in the HKL2000 software. BTW in the original implementation of the kernels, the kernel was centered incorrectly and produced a shift in the image (the center of the kernel should be at the corners of the image, not the center of the image).
other comments:
-
note that blob-finder and peak-finder were refactored to incorporate a ProgressHandler, which can be viewed in the GUI via ProgressView. This allows detailed feedback in the GUI during long calculations. After the merge it would probably be a good idea to incorporate it into other time-consuming calculations (such as indexing or unit cell search)
-
JobHandler.cpp gives simple classes that allow calculations to be run in a separate thread so that the GUI remains responsive. The can be connected to a ProgressHandler/ProgressView so that the user gets feedback about the progress of the calculation.
-
Right, we should propagate your implementation to other time-consuming calculations. BTW, when checking out conv-merge branch and building I get an error when compiling blosc-snappy filter due to unrecognized --std=c++14 flag. However, looking at the CMakeLists.txt files of the project and especially the ones of cblosc, I do not see any deep change there especially for auto-detection of GNU compiler flag. I still work on ubuntu 14.04 with gcc 4.8.4. Would you have any idea ?
Edited by eric pellegriniCan you share the data with me? I have only biodiff and D19 data 094542. On 094542 it finds the peaks just as in the old version. If you use the default values in peak find it should behave exactly as in the old version, there is no filter applied unless you manually specify it (and for D19, you have to play with the values a bit to get a good result from the filter)
Added 1 commit:
- 4e4b9477 - bugfix in peak find dialog (crash if no data is viewed in current scene)
In current commit, with default values in peak find dialog, I get 62 peaks for D19 dataset 094542. Please confirm whether you get the same result (and whether this is correct!)
There is another bug, a segfault when the program exits due to some memory being freed twice. This happens only at exit so it is not a fatal bug. I am not sure whether I introduced it, or whether it also exists in master branch. I will try to track it down.
Using the master branch and the default values (3.00 for the bkg threshold and 0.997 for the confidence interval), I find 68 peaks that you will find sorted by decreasing intensity in the enclosed file.94542_peaks.txt
This is fixed now, I find 68 peaks as before. The problem was that median is treated as a double in new branch instead of integer as before, so we get inconsistent results unless we truncate.
Before merging, there is still a segfault at program exit which I am trying to track down. It doesn't occur in master to it's a consequence of something in this branch.
Added 1 commit:
- eea46055 - fixed segfault and memory leak reported by valgrind