Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added connected components #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
265 changes: 265 additions & 0 deletions src/connectComp.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
#include <stdio.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename files to edge.cpp and edge.hpp for now

#include <stdlib.h>
#include <math.h>

#include <vector>
#include <iostream>

#include <unordered_map>

#include <unordered_set>
#include <string.h>
#include <cassert>
#include <cstdlib>

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#include your .hpp file here. You'll then notice that there is a double definition, but the below comments will fix that.

namespace found {

struct Point {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Vec2 instead, which is found in attitude-utils.hpp

int x;
int y;
};

struct Edge {
int magnitude; // number of points in this edge
std::vector<Point> points;
// int Xmin, Xmax, Ymin, Ymax;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding these metadata fields in. It may be useful in the future to distinguish between edges that might be part of Earth (in which case, one of these fields should be near the border), and others. Also, relocate the Edge struct to the style.hpp file.

};

std::vector<Edge> NaiveConnectedComponent(unsigned char *image, int imageWidth, int imageHeight) { // const??
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you plan on making a better version, it might be better to name this just as ConnectedComponents or such

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may go ahead and const the image field, but you should do this at both the pointer level and the value level. This will make sure that you cannot change neither the value nor the reference stored by this pointer. If its inconvenient, it does not need to be done though.

int threshold = 3; // over threshold means possible components exist
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine for now, I will put in an edit to this particular line when Karuna pushes her algorithm. In the meantime, please place a TODO: Edit this for compatibility with edge filtering comment above this


// Tells us if certian numbers in the below array are part of the same component
std::unordered_map<int, int> equivalencies;

// make an array with the same dimensions of the image, but it shows connected components using numbers
unsigned char *edges = (unsigned char *) std::malloc(imageWidth * imageHeight * sizeof(unsigned char));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we might be better off with a stack allocated array instead, since it isn't really relevant across the entire program.

edges[0] = (image[0] > threshold) ? 1 : 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above, comment with: TODO: Edit this for compatibility with edge filtering

std::cout << static_cast<int>(edges[0]) << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I bet this print statement shouldn't be here. Eventually, when you're done debugging and testing, I will ask that you remove all print statements that shouldn't exist in the program, as well as "commented-out" code.

int L = edges[0];

// for (int i = 0; i < imageHeight; ++i) {
// for (int j = 0; j < imageWidth; ++j) {
// std::cout << "(" << static_cast<int>(edges[i * imageWidth + j]) << ") ";
// }
// std::cout << std::endl;
// }


// Initial labeling of centroids
for(long i = 1; i < imageHeight * imageWidth; i++) {
// This is specific to the centroiding algo, which tests if this is even part of a centroid
if(image[i] > threshold) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above, comment with: TODO: Edit this for compatibility with edge filtering


// The indicies of the entries to the left and above i
int up = i - imageWidth;
int left = i - 1;
int upLeft = i - imageWidth - 1;
int upRight = i - imageWidth + 1;

// Tests to see if the upper or left entry is a valid component (part of a star aka it was initialized
// with a L value at some point)

// Is there concern about negative indexing?
bool leftEq = edges[left] != 0;
bool upEq = edges[up] != 0;

std::cout << "(" << leftEq << "," << upEq << ")";
bool upLeftEq = edges[upLeft] != 0;
bool upRightEq = edges[upRight] != 0;

// Are we assuming image is of two colors?

if(i / imageWidth == 0) { // If i is part of a top pixel, test only the left pixel
if(leftEq) { // If the left pixel is part of a centroid, it must be part of the same star as i, if not, mark with new L value
edges[i] = edges[left];
} else {
edges[i] = ++L;
}
} else if(i % imageWidth == 0) { // If i is part of a left pixel, test the upper pixel and top right pixel
if(upEq && upRightEq && edges[up] == edges[upRight]) { // If the upper and top right pixel is part of a centroid, it must be part of the same star as i, if not, mark with new L value
edges[i] = edges[up];
} else if(upEq && upRightEq && edges[up] != edges[upRight]) {
edges[i] = std::min(edges[up], edges[upRight]);
equivalencies.insert(std::pair<int, int>(int(std::max(edges[up], edges[upRight])), int(edges[i])));
assert(equivalencies.find(edges[i]) == equivalencies.end());
} else if(upEq) { // top pixel is part of
edges[i] = edges[up];
} else if(upRightEq) { // top right pixel is part of
edges[i] = edges[upRight];
}
else {
edges[i] = ++L;
}
} else if (i + 1 % imageWidth == 0) { // If i is part of a right pixel, test only left and upper
if(upLeftEq) {
edges[i] = edges[upLeft];
} else if(leftEq && upEq && edges[left] == edges[up]) { // If the top and left pixels are part of centroid with the same L value, i is connected to them with the same L value
edges[i] = edges[left];
} else if(leftEq && upEq && edges[left] != edges[up]) { // The above, but if the left and top pixels have different L values, those different values are also part of the same centroid
edges[i] = std::min(edges[left], edges[up]);
equivalencies.insert(std::pair<int, int>(int(std::max(edges[left], edges[up])), int(edges[i])));
assert(equivalencies.find(edges[i]) == equivalencies.end());
} else if(leftEq && upRightEq && edges[left] == edges[upRight]) { // left and top right
edges[i] = edges[left];
} else if(leftEq && upRightEq && edges[left] != edges[upRight]) { // left and top right
edges[i] = std::min(edges[left], edges[upRight]);
equivalencies.insert(std::pair<int, int>(int(std::max(edges[left], edges[upRight])), int(edges[i])));
assert(equivalencies.find(edges[i]) == equivalencies.end());
} else if(upLeftEq && upRightEq && edges[upLeft] == edges[upRight]) { // top left and top right
edges[i] = edges[upLeft];
} else if(upLeftEq && upRightEq && edges[upLeft] != edges[upRight]) { // top left and top right
edges[i] = std::min(edges[upLeft], edges[upRight]);
equivalencies.insert(std::pair<int, int>(int(std::max(edges[upLeft], edges[upRight])), int(edges[i])));
assert(equivalencies.find(edges[i]) == equivalencies.end());
} else if(leftEq) { // From here, we get a copy of the first two if/else if statements
edges[i] = edges[left];
} else if(upEq) {
edges[i] = edges[up];
} else {
edges[i] = ++L;
}
} else { // Tests for general case
if(upLeftEq) {
edges[i] = edges[upLeft];
} else if(upRightEq) {
edges[i] = edges[upRight];
} else if(leftEq && upEq && edges[left] == edges[up]) { // If the top and left pixels are part of centroid with the same L value, i is connected to them with the same L value
edges[i] = edges[left];
} else if(leftEq && upEq && edges[left] != edges[up]) { // The above, but if the left and top pixels have different L values, those different values are also part of the same centroid
edges[i] = std::min(edges[left], edges[up]);
equivalencies.insert(std::pair<int, int>(int(std::max(edges[left], edges[up])), int(edges[i])));
assert(equivalencies.find(edges[i]) == equivalencies.end());
} else if(leftEq) { // From here, we get a copy of the first two if/else if statements
edges[i] = edges[left];
} else if(upEq) {
edges[i] = edges[up];
} else {
edges[i] = ++L;
}
}
}
else {
edges[i] = 0;
}
}
std::cout << std::endl;

for (int i = 0; i < imageHeight; ++i) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another debugging tool here

for (int j = 0; j < imageWidth; ++j) {
std::cout << "(" << static_cast<int>(edges[i * imageWidth + j]) << ") ";
}
std::cout << std::endl;
}

// Get statistics of each star. This is when we process the above info
// into groups based on the pixels having the same number (or equivalent numebers via the map)
// params is a mapping of these centroid numbers (L values) to each "group" of pixels with statistics
// shown in Centroid params.
std::unordered_map<int, Edge> params; // Star # to param
for(int i = 0; i < imageWidth * imageHeight; i++) {
if(edges[i] != 0) {
int edgeNumber = equivalencies.find(edges[i]) == equivalencies.end() ?
edges[i] : equivalencies.find(edges[i]) -> second;
if(params.find(edgeNumber) == params.end()) { // If the entry with the L value given at stars[i] is not yet in params, then initialze one
Edge e;
std::vector<Point> point;
e.magnitude = 0;
e.points = point;
//p.xMax = 0;
//p.xMin = imageWidth;
//p.yMax = 0;
//p.yMin = imageHeight;
//e.isValid = true;
params.insert(std::pair<int, Edge>(edgeNumber, e));
}
// Obtain and update the statistics for the centroid corresponding to the L value at stars[i] or the equivalent L value
Edge *edge_ptr = &params.at(edgeNumber);
int row = i / imageWidth;
int col = i % imageWidth;
// centroid_ptr -> xCoordMagSum += col * image[i];
// centroid_ptr -> yCoordMagSum += row * image[i];
edge_ptr -> magnitude++;
Point p;
p.x = row;
p.y = col;
edge_ptr -> points.push_back(p);
// centroid_ptr -> magSum += image[i];
// if(col > centroid_ptr -> xMax) {
// centroid_ptr -> xMax = col;
// }
// if(col < centroid_ptr -> xMin) {
// centroid_ptr -> xMin = col;
// }
// if(row > centroid_ptr -> yMax) {
// centroid_ptr -> yMax = row;
// }
// if(row < centroid_ptr -> yMin) {
// centroid_ptr -> yMin = row;
// }
// if (i % imageWidth == 0 || i % imageWidth == imageWidth - 1 || i / imageWidth == 0 ||
// i / imageWidth == imageHeight - 1) {
// centroid_ptr -> isValid = false;
// }

}
}


std::vector<Edge> connectedPoints;

for (const auto& pair : params) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use the auto keyword unless it is extremely inconvenient (i.e. for a variable that holds a function)

connectedPoints.push_back(pair.second);
}

return connectedPoints;
}

}

// // Function to generate a random unsigned char value between min and max (inclusive)
// unsigned char getRandomChar(unsigned char min, unsigned char max) {
// return static_cast<unsigned char>(rand() % (max - min + 1) + min);
// }

// // Main function to test the above function
// int main() {
// // Seed the random number generator
// srand(static_cast<unsigned int>(time(0)));

// // Example 2D array (3x3)
// int height = 5;
// int width = 5;
// // Allocate memory for the 2D array
// unsigned char* image = new unsigned char[height * width];

// // Fill the array with random values and print it
// for (int i = 0; i < height; ++i) {
// for (int j = 0; j < width; ++j) {
// image[i * width + j] = getRandomChar(0, 4);
// std::cout << "(" << static_cast<int>(image[i * width + j]) << ") ";
// }
// std::cout << std::endl;
// }

// // Call the function and get the result
// std::vector<Edge> flattened = NaiveConnectedComponent(image, height, width);

// std::size_t length = flattened.size();

// std::cout << "The length of the vector is: " << length << std::endl;

// // Print the resulting vector
// std::cout << "Flattened array: ";
// for (Edge value : flattened) {
// for (Point p : value.points) {
// std::cout << "(" << p.x << "," << p.y << ") ";
// }
// std::cout << std::endl;
// }
// std::cout << std::endl;

// // Clean up the allocated memory
// delete[] image;

// return 0;
// }
25 changes: 25 additions & 0 deletions src/connectComp.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#ifndef CONNECTCOMP_H
#define CONNECTCOMP_H

#include <vector>
#include <unordered_map>
#include <string>

namespace found {

struct Point {
int x;
int y;
};

struct Edge {
int magnitude; // number of points in this edge
std::vector<Point> points;
};

// Function prototype
std::vector<Edge> NaiveConnectedComponent(unsigned char *image, int imageWidth, int imageHeight);
Copy link
Collaborator

@nguy8tri nguy8tri Jan 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put a more descriptive function comment here. There should be:

  1. General Description of the Funciton
  2. Parameter Description
  3. Outputs
  4. Any modifications on the parameters
  5. Preconditions
  6. (optional) Postconditions

You can use @ tags for this. More info here: https://darkognu.eu/programming/tutorials/doxygen_tutorial_cpp/.


}

#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you have completed tests on this function, so I will need you to formalize that in the test suite. That can be found in "found/test/test.cpp" (No .hpp file needed here). example.cpp will help you figure out how to write tests using C++'s CHECK framework (I'm open to other test suites if its better by the way).