Skip to content

Credit the moved node's volume to the community it moved into - #178

Open
arpitjain099 wants to merge 1 commit into
pnnl:masterfrom
arpitjain099:fix/modularity-last-step-volume
Open

arpitjain099 wants to merge 1 commit into
pnnl:masterfrom
arpitjain099:fix/modularity-last-step-volume

Conversation

@arpitjain099

Copy link
Copy Markdown

_last_step_weighted and _last_step_unweighted search for the best community for a node, then apply the move:

dct_A[v] = best
VolA[m] += str_v
VolA[dct_A_v] -= str_v

m is the loop variable from the search above, so after the loop it holds whichever candidate happened to come last out of set([i for x in L for i in x]) - {dct_A_v}, not the community the node actually moved into. The node goes to best, but its volume is added to m. From then on the per-community volumes are wrong, and since the degree tax for every later move is computed from VolA, the errors compound over the sweep.

Changed both to VolA[best].

I measured it on 24 random hypergraphs (30 nodes, 40 edges of size 2 to 5, singleton start, delta=0.01), comparing the modularity of the returned partition. 20 of 24 improved, mean 0.245 to 0.309. Individual seeds move both ways, which is expected of a greedy heuristic, and the run to run numbers wobble a little because the candidate loop iterates a set, but the direction is consistent.

I did not add a test for it: for the same reason the results are not reproducible enough for a threshold assertion, and I did not want to add a flaky one. Happy to add something if you have a preference for how.

While reading this I noticed a separate issue in the same file, not touched here: VolA and Ctr/ctr_sizes are allocated with np.repeat(0, n), which is int64, and then accumulate H.edges[e].weight. Fractional weights are truncated on the way in, so a hypergraph with weights below 1 gets zero volume.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant