Skip to content

fix: Numpy sum error #104

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.

Sign up for GitHub

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

Merged
cbueth merged 2 commits into from
Mar 11, 2026
Merged

fix: Numpy sum error #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ecb3baf
updated code with bug
juanfonsecaLS1 Mar 9, 2026
adb63f7
fixes TypeError in utils
juanfonsecaLS1 Mar 9, 2026
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
4 changes: 2 additions & 2 deletions superblockify/metrics/measures.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,9 @@ def calculate_coverage(partitioner, weight):
if partitioner.graph.number_of_edges() == partitioner.sparsified.number_of_edges():
return 0

return 1 - npsum(
return 1 - sum(
Copy link
Copy Markdown
Collaborator

@cbueth cbueth Mar 9, 2026

Choose a reason for hiding this comment

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

This now uses the python integrated sum? If we are not using numpy's sum, then the import from numpy import sum as npsum should be removed, as it is not used anywhere else in this file. I'm confused why this is erroring here, while using sum by numpy sin other places of this package still works.
I'd say if the CI works there is no need to dig into the details.

Copy link
Copy Markdown
Collaborator Author

@juanfonsecaLS1 juanfonsecaLS1 Mar 9, 2026

Choose a reason for hiding this comment

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

Yes, I went for the simpler way as suggested in the error line I was getting. I did not remove the import as the npsum is still used in calculate_global_efficiency

def calculate_global_efficiency(distance_matrix, measure1, measure2):
    r"""Calculate the global efficiency for the given network measures.

    The global efficiency is the ratio between the sums of the inverses of the
    distances of the two network measures.

    If any of the distances is 0 or infinite, it is ignored in the calculation.

    Parameters
    ----------
    distance_matrix : dict
        The distance matrix for the network measures, as returned by instance attribute
        :attr:`superblockify.metrics.metric.Metric.distance_matrix`
    measure1 : str
        The first network measure
    measure2 : str
        The second network measure

    Returns
    -------
    float
        The global efficiency of the network measures

    Notes
    -----
    .. math::

         E_{\text{glob},S/E}=\frac{\sum_{i \neq j}\frac{1}{d_S(i, j)}}
         {\sum_{i \neq j} \frac{1}{d_E(i, j)}}
    """

    dist1, dist2 = _network_measures_filtered_flattened(
        distance_matrix, measure1, measure2
    )

    # Calculate the global efficiency as the ratio between the sums of the inverses
    return npsum(1 / dist1) / npsum(1 / dist2)

Copy link
Copy Markdown
Collaborator

@cbueth cbueth Mar 11, 2026

Choose a reason for hiding this comment

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

Good point, I did not see it there. Let's see what the second CI run now says.

d[weight] for u, v, d in partitioner.sparsified.edges(data=True)
) / npsum(d[weight] for u, v, d in partitioner.graph.edges(data=True))
) / sum(d[weight] for u, v, d in partitioner.graph.edges(data=True))


def rel_increase(value_i, value_j):
Expand Down
6 changes: 4 additions & 2 deletions superblockify/partitioning/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,12 @@ def show_highway_stats(graph):
"Dtype counts (type, count, proportion): \n%s", dtype_counts.to_string()
)
# Warning if 'str' is underrepresented
if dtype_counts.loc[dtype_counts.index == str, "proportion"].values < 0.98:
str_proportion_series = dtype_counts.loc[dtype_counts.index == str, "proportion"]
str_proportion = str_proportion_series.iloc[0] if not str_proportion_series.empty else 0.0
if str_proportion < 0.98:
logger.warning(
"The dtype of the 'highway' attribute is not 'str' in %d%% of the edges.",
(1 - dtype_counts.loc[dtype_counts.index == str, "proportion"]) * 100,
int((1 - str_proportion) * 100),
)


Expand Down
Loading