sandialabs / seat-qgis-plugin

Spatial Environmental Assessment Toolkit (SEAT) QGIS Plugin
https://sandialabs.github.io/seat-qgis-plugin/
GNU General Public License v3.0
1 stars 7 forks source link

Bug: Power Module `centroid_diffs` #32

Open ssolson opened 10 months ago

ssolson commented 10 months ago

@tnelson-integral I believe there is a bug in centroid_diffs calculation.

https://github.com/sandialabs/seat-qgis-plugin/blob/d04f83e5584b713f5e9ba728759a9a1dd54461b0/seat/power_module.py#L125-L147

I created the following test using the euclidean distance to calculate the distance

    def test_centroid_diffs(self):
        """
        Test the centroid_diffs function, considering the first value in each centroid as an index.
        """
        # Prepare test inputs
        test_centroids = np.array([
            [0, 2.0, 2.0],  # Index 0, Coordinates (2.0, 2.0)
            [1, 5.0, 5.0],  # Index 1, Coordinates (5.0, 5.0)
            [2, 7.0, 8.0]   # Index 2, Coordinates (7.0, 8.0)
        ])
        test_centroid = np.array([3, 6.0, 6.0])  # Index 3, Coordinates (6.0, 6.0)

        # Expected output: Closest centroid to test_centroid
        def calculate_expected_pair(centroids, centroid):
            # Calculate Euclidean distances, excluding the index
            distances = np.sqrt(np.sum((centroids[:, 1:] - centroid[1:])**2, axis=1))

            # Find the index of the centroid with the minimum distance
            closest_index = np.argmin(distances)

            # Return the expected pair: [index of test_centroid, index of closest centroid in test_centroids]
            return [int(centroid[0]), int(centroids[closest_index, 0])]

        expected_pair = self.calculate_expected_pair(test_centroids, test_centroid)

        # Call the function
        result_pair = pm.centroid_diffs(test_centroids, test_centroid)

        # Assert the result
        self.assertEqual(result_pair, expected_pair, "The identified closest centroid pair is incorrect.")

The expect pair differs: AssertionError: Lists differ: [3, 0] != [3, 1]

Am I missing something about the data and the way it should be handled?

ssolson commented 10 months ago

I think this would be the correct implementation:

def centroid_diffs(Centroids, centroid):
    # Calculate Euclidean distances
    distances = np.sqrt(np.sum((Centroids[:, 1:] - centroid[1:])**2, axis=1))

    # Find the index of the centroid with the minimum distance
    min_arg = np.argmin(distances)

    # Return the pair: index of the single centroid and the index of the closest centroid
    pair = [int(centroid[0]), int(Centroids[min_arg, 0])]
    return pair
tnelson-integral commented 10 months ago

While the current method works for the tutorial dataset (see image), it is not robust for different configurations. The suggested method is more robust and fixes this issue. centroid diffs