diff --git a/src/graph/algorithms/leiden/partition.ts b/src/graph/algorithms/leiden/partition.ts index 69fd9208..54cc6e11 100644 --- a/src/graph/algorithms/leiden/partition.ts +++ b/src/graph/algorithms/leiden/partition.ts @@ -9,7 +9,7 @@ import type { GraphAdapter } from './adapter.js'; import { accumulateInternalEdgeWeights, accumulateNodeAggregates } from './aggregate-helpers.js'; -import { fget, fgetOrZero, iget, u8get } from './typed-array-helpers.js'; +import { fget, iget, u8get } from './typed-array-helpers.js'; export interface CompactOptions { keepOldOrder?: boolean; @@ -29,14 +29,10 @@ export interface Partition { resizeCommunities(newCount: number): void; initializeAggregates(): void; accumulateNeighborCommunityEdgeWeights(v: number): number; - getCandidateCommunityCount(): number; getCandidateCommunityAt(i: number): number; getNeighborEdgeWeightToCommunity(c: number): number; getOutEdgeWeightToCommunity(c: number): number; getInEdgeWeightFromCommunity(c: number): number; - deltaModularityUndirected(v: number, newC: number, gamma?: number): number; - deltaModularityDirected(v: number, newC: number, gamma?: number): number; - deltaCPM(v: number, newC: number, gamma?: number): number; moveNodeToCommunity(v: number, newC: number): boolean; compactCommunityIds(opts?: CompactOptions): void; getCommunityMembers(): number[][]; @@ -229,163 +225,6 @@ function accumulateNeighborWeights(s: PartitionState, v: number): number { return s.candidateCommunityCount; } -/* ------------------------------------------------------------------ */ -/* Extracted: modularity delta computations */ -/* ------------------------------------------------------------------ */ - -function computeDeltaModularityUndirected( - s: PartitionState, - v: number, - newC: number, - gamma: number = 1.0, -): number { - const oldC: number = iget(s.nodeCommunity, v); - if (newC === oldC) return 0; - const twoM: number = s.graph.totalWeight; - const strengthV: number = fget(s.graph.strengthOut, v); - const weightToNew: number = - newC < s.neighborEdgeWeightToCommunity.length - ? fget(s.neighborEdgeWeightToCommunity, newC) || 0 - : 0; - const weightToOld: number = fget(s.neighborEdgeWeightToCommunity, oldC) || 0; - const totalStrengthNew: number = - newC < s.communityTotalStrength.length ? fget(s.communityTotalStrength, newC) : 0; - const totalStrengthOld: number = fget(s.communityTotalStrength, oldC); - const gain_remove: number = -( - weightToOld / twoM - - (gamma * (strengthV * totalStrengthOld)) / (twoM * twoM) - ); - const gain_add: number = - weightToNew / twoM - (gamma * (strengthV * totalStrengthNew)) / (twoM * twoM); - return gain_remove + gain_add; -} - -/** - * Directed delta-modularity edge-weight terms: in/out edge weight from `v` to - * `newC`/`oldC`, read via the shared bounds-checked `fgetOrZero` (see its - * doc comment for why `|| 0` is safe/redundant here but retained anyway). - */ -function computeDirectedEdgeWeightTerms( - s: PartitionState, - newC: number, - oldC: number, -): { inFromNew: number; outToNew: number; inFromOld: number; outToOld: number } { - return { - inFromNew: fgetOrZero(s.inEdgeWeightFromCommunity, newC), - outToNew: fgetOrZero(s.outEdgeWeightToCommunity, newC), - inFromOld: fgetOrZero(s.inEdgeWeightFromCommunity, oldC), - outToOld: fgetOrZero(s.outEdgeWeightToCommunity, oldC), - }; -} - -/** - * Directed delta-modularity community-strength terms: total in/out strength - * of `newC`/`oldC`, read via the shared bounds-checked `fgetOrZero`. - */ -function computeDirectedStrengthTerms( - s: PartitionState, - newC: number, - oldC: number, -): { - totalInStrengthNew: number; - totalOutStrengthNew: number; - totalInStrengthOld: number; - totalOutStrengthOld: number; -} { - return { - totalInStrengthNew: fgetOrZero(s.communityTotalInStrength, newC), - totalOutStrengthNew: fgetOrZero(s.communityTotalOutStrength, newC), - totalInStrengthOld: fgetOrZero(s.communityTotalInStrength, oldC), - totalOutStrengthOld: fgetOrZero(s.communityTotalOutStrength, oldC), - }; -} - -function computeDeltaModularityDirected( - s: PartitionState, - v: number, - newC: number, - gamma: number = 1.0, -): number { - const oldC: number = iget(s.nodeCommunity, v); - if (newC === oldC) return 0; - const totalEdgeWeight: number = s.graph.totalWeight; - const strengthOutV: number = fget(s.graph.strengthOut, v); - const strengthInV: number = fget(s.graph.strengthIn, v); - const { inFromNew, outToNew, inFromOld, outToOld } = computeDirectedEdgeWeightTerms( - s, - newC, - oldC, - ); - const { totalInStrengthNew, totalOutStrengthNew, totalInStrengthOld, totalOutStrengthOld } = - computeDirectedStrengthTerms(s, newC, oldC); - // Self-loop correction + constant term (see modularity.ts diffModularityDirected) - const selfW: number = fget(s.graph.selfLoop, v) || 0; - const deltaInternal: number = - (inFromNew + outToNew - inFromOld - outToOld + 2 * selfW) / totalEdgeWeight; - const deltaExpected: number = - (gamma * - (strengthOutV * (totalInStrengthNew - totalInStrengthOld) + - strengthInV * (totalOutStrengthNew - totalOutStrengthOld) + - 2 * strengthOutV * strengthInV)) / - (totalEdgeWeight * totalEdgeWeight); - return deltaInternal - deltaExpected; -} - -/** computeCpmEdgeWeights — directed branch: in+out weight, plus self-loop correction. */ -function computeCpmEdgeWeightsDirected( - s: PartitionState, - v: number, - oldC: number, - newC: number, -): { wOld: number; wNew: number; selfCorrection: number } { - const wOld: number = - (fget(s.outEdgeWeightToCommunity, oldC) || 0) + (fget(s.inEdgeWeightFromCommunity, oldC) || 0); - const wNew: number = - newC < s.outEdgeWeightToCommunity.length - ? (fget(s.outEdgeWeightToCommunity, newC) || 0) + - (fget(s.inEdgeWeightFromCommunity, newC) || 0) - : 0; - // Self-loop correction (see cpm.ts diffCPM) - const selfCorrection: number = 2 * (fget(s.graph.selfLoop, v) || 0); - return { wOld, wNew, selfCorrection }; -} - -/** computeCpmEdgeWeights — undirected branch: single neighbor-weight-to-community value. */ -function computeCpmEdgeWeightsUndirected( - s: PartitionState, - oldC: number, - newC: number, -): { wOld: number; wNew: number; selfCorrection: number } { - const wOld: number = fget(s.neighborEdgeWeightToCommunity, oldC) || 0; - const wNew: number = - newC < s.neighborEdgeWeightToCommunity.length - ? fget(s.neighborEdgeWeightToCommunity, newC) || 0 - : 0; - return { wOld, wNew, selfCorrection: 0 }; -} - -/** Directed/undirected edge-weight-to-community split used by computeDeltaCPM. */ -function computeCpmEdgeWeights( - s: PartitionState, - v: number, - oldC: number, - newC: number, -): { wOld: number; wNew: number; selfCorrection: number } { - return s.graph.directed - ? computeCpmEdgeWeightsDirected(s, v, oldC, newC) - : computeCpmEdgeWeightsUndirected(s, oldC, newC); -} - -function computeDeltaCPM(s: PartitionState, v: number, newC: number, gamma: number = 1.0): number { - const oldC: number = iget(s.nodeCommunity, v); - if (newC === oldC) return 0; - const { wOld: w_old, wNew: w_new, selfCorrection } = computeCpmEdgeWeights(s, v, oldC, newC); - const nodeSz: number = fget(s.graph.size, v) || 1; - const sizeOld: number = fget(s.communityTotalSize, oldC) || 0; - const sizeNew: number = newC < s.communityTotalSize.length ? fget(s.communityTotalSize, newC) : 0; - return w_new - w_old + selfCorrection - gamma * nodeSz * (sizeNew - sizeOld + nodeSz); -} - /* ------------------------------------------------------------------ */ /* Extracted: node move */ /* ------------------------------------------------------------------ */ @@ -594,17 +433,11 @@ export function makePartition(graph: GraphAdapter, options: MakePartitionOptions }, initializeAggregates: () => initAggregates(s), accumulateNeighborCommunityEdgeWeights: (v: number) => accumulateNeighborWeights(s, v), - getCandidateCommunityCount: (): number => s.candidateCommunityCount, getCandidateCommunityAt: (i: number): number => iget(s.candidateCommunities, i), getNeighborEdgeWeightToCommunity: (c: number): number => fget(s.neighborEdgeWeightToCommunity, c) || 0, getOutEdgeWeightToCommunity: (c: number): number => fget(s.outEdgeWeightToCommunity, c) || 0, getInEdgeWeightFromCommunity: (c: number): number => fget(s.inEdgeWeightFromCommunity, c) || 0, - deltaModularityUndirected: (v: number, newC: number, gamma?: number) => - computeDeltaModularityUndirected(s, v, newC, gamma), - deltaModularityDirected: (v: number, newC: number, gamma?: number) => - computeDeltaModularityDirected(s, v, newC, gamma), - deltaCPM: (v: number, newC: number, gamma?: number) => computeDeltaCPM(s, v, newC, gamma), moveNodeToCommunity: (v: number, newC: number) => moveNode(s, v, newC), compactCommunityIds: (opts?: CompactOptions) => compactIds(s, opts), getCommunityMembers(): number[][] { diff --git a/src/graph/algorithms/leiden/typed-array-helpers.ts b/src/graph/algorithms/leiden/typed-array-helpers.ts index 698446bc..ce3ef58a 100644 --- a/src/graph/algorithms/leiden/typed-array-helpers.ts +++ b/src/graph/algorithms/leiden/typed-array-helpers.ts @@ -22,33 +22,6 @@ export function u8get(a: Uint8Array, i: number): number { return a[i] as number; } -/** - * Bounds-checked community-accumulator read: `i < a.length ? (fget(a, i) || 0) : 0`. - * - * Community ids are sometimes probed before the community itself has been - * grown into a given per-community accumulator array (e.g. a brand-new - * `newC` about to receive its first member) — the bounds check treats that - * as a not-yet-existing community contributing zero, matching - * `getNeighborEdgeWeightToCommunity`/`getOutEdgeWeightToCommunity`/ - * `getInEdgeWeightFromCommunity` in partition.ts, which every other reader - * of these arrays already goes through. - * - * The `|| 0` is not reachable today: every array this is used with in this - * codebase (communityTotalStrength/In/Out, neighborEdgeWeightToCommunity, - * outEdgeWeightToCommunity, inEdgeWeightFromCommunity) is a dense, - * zero-initialized Float64Array populated purely by +=/-= over edge weights - * and node strengths that are scrubbed of NaN/undefined at - * `makeGraphAdapter` construction time (`+linkWeight(attrs) || 0` / - * `+nodeSize(attrs) || 0` in adapter.ts) — so a bare bounds-checked `fget` - * would return the identical value. Kept for defense-in-depth against a - * future change to that invariant, and so callers reading either array - * family use one consistent, already-safe accessor instead of - * hand-rolling the same ternary+`||` per call site (see issue #1755). - */ -export function fgetOrZero(a: Float64Array, i: number): number { - return i < a.length ? fget(a, i) || 0 : 0; -} - /** In-place compound addition: `a[i] += v`, safe under noUncheckedIndexedAccess. */ export function taAdd(a: Float64Array, i: number, v: number): void { a[i] = fget(a, i) + v; diff --git a/tests/graph/algorithms/leiden.test.ts b/tests/graph/algorithms/leiden.test.ts index 37f22e26..69aa6e2e 100644 --- a/tests/graph/algorithms/leiden.test.ts +++ b/tests/graph/algorithms/leiden.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from 'vitest'; import { detectClusters } from '../../../src/graph/algorithms/leiden/index.js'; -import { fgetOrZero } from '../../../src/graph/algorithms/leiden/typed-array-helpers.js'; import { CodeGraph } from '../../../src/graph/model.js'; // ─── Helpers ────────────────────────────────────────────────────────── @@ -653,48 +652,18 @@ describe('community connectivity', () => { }); }); -// ─── fgetOrZero (typed-array-helpers) ────────────────────────────────── -// -// Shared bounds-checked accessor extracted from computeDeltaModularityDirected -// (issue #1755) to unify the "newC < arr.length ? fget(arr, newC) [|| 0] : 0" -// pattern that previously appeared with an inconsistent `|| 0` across the -// edge-weight-to-community arrays vs. the community-strength-total arrays. - -describe('fgetOrZero (typed-array-helpers)', () => { - it('returns the stored value for an in-bounds index', () => { - const a = new Float64Array([1, 2, 3]); - expect(fgetOrZero(a, 1)).toBe(2); - }); - - it('returns 0 for an index at or beyond the array length (not-yet-existing community)', () => { - const a = new Float64Array([1, 2, 3]); - expect(fgetOrZero(a, 3)).toBe(0); - expect(fgetOrZero(a, 100)).toBe(0); - }); - - it('returns a plain 0 for a legitimate zero entry, same as a bare bounds-checked read', () => { - const a = new Float64Array([0, 5]); - expect(fgetOrZero(a, 0)).toBe(0); - }); - - it('squashes a stray NaN to 0 (defense-in-depth guard; not reachable via current callers)', () => { - const a = new Float64Array([Number.NaN, 5]); - expect(Number.isNaN(fgetOrZero(a, 0))).toBe(false); - expect(fgetOrZero(a, 0)).toBe(0); - }); -}); - // ─── Directed modularity delta — exact regression values ─────────────── // // Pins the exact quality()/community-assignment output of the two existing -// "directed modularity" / "directed self-loops" fixtures above, computed -// through computeDeltaModularityDirected's fgetOrZero-based reads (issue -// #1755). These exact values were verified byte-for-byte identical against -// the pre-refactor implementation (four independent bounds-check-and-`||0` -// expressions) on this same seed. A future change that re-diverges the -// `|| 0` handling between the edge-weight and community-strength array -// families (or otherwise perturbs the delta-modularity formula) would shift -// these floating-point values and fail this test. +// "directed modularity" / "directed self-loops" fixtures above. These go +// through the live directed-modularity delta path — diffModularityDirected +// in modularity.ts, invoked via computeQualityGain (optimiser.ts) during the +// Leiden local-move and refinement phases — not Partition's deltaModularityDirected +// interface method, which issue #1770 found to be unreachable dead code (no +// caller ever invokes partition.deltaModularityDirected(...)) and removed. +// A future change that perturbs diffModularityDirected's formula or its +// bounds-checked reads would shift these floating-point values and fail +// this test. describe('directed modularity delta — exact regression', () => { it('matches the pinned quality/assignment for the two-triangle directed graph', () => {