Least Squares Gaussian Fit + Gaussian Grid centroiding algos #89
Conversation
|
Tab width should be 4 spaces, as in the rest of the codebase |
|
|
|
for reference, this is how --centroid-algo cog looks like in the same area |
zeddie888
changed the title
zeddie888
changed the title
Makefile
Outdated
| # bash ./test/scripts/pyramid-incorrect.sh | ||
| bash ./test/scripts/readme-examples-test.sh | ||
| bash ./test/scripts/pyramid-incorrect.sh | ||
| # bash ./test/scripts/readme-examples-test.sh |
There was a problem hiding this comment.
this stuff shouldn't be changed
| Stars Go(unsigned char *image, int imageWidth, int imageHeight) const override; | ||
| }; | ||
|
|
||
| class GaussianGrid : public CentroidAlgorithm { |
|
|
||
| } | ||
| /// Get value of pixel at (x, y) in image with width=w | ||
| int Get(int x, int y, const unsigned char *image, int w); |
There was a problem hiding this comment.
functions with such short undescriptive names sholudn't be in hpp file, they could conflict with other names. Really we shouldn't have any functions anywhere named Get.
If there are some functions you only need to access for testing purposes, you can create a centroiders-private.hpp file that contains declarations for these. That's what I did with star-id-private.hpp recently in some pyramid branch.
src/centroiders.cpp
Outdated
| /// Prediction of 2D Gaussian model for 2D LSGF method | ||
| static float FitModel2D(float x, float y, float a, float xb, float yb, float sigmaX, float sigmaY); | ||
|
|
||
| typedef std::vector<int> Point; |
|
The tests do not compile. |
| } | ||
|
|
||
| // Cool, our flood is done | ||
| res.push_back({x0, y0, floodSize, fp.xMax - fp.xMin, fp.yMax - fp.yMin}); |
There was a problem hiding this comment.
res is a std::vector<std::vector<int>>. However, every element has the same structure, so there's no reason to use a vector -- it should be a vector of structs. It's okay to make single-use structs. (And while I like to use structs instead of tuples, a tuple is still better than a vector here).
(I'm going to fix this myself because I'm rewriting a whole bunch of centroid stuff right now, just thought you should know)
No description provided.