core: Graph operator composition + modularityScore — consolidated (supersedes DIRTY #319 + #322)

[AceHack]

Closes positional-rebase-conflict loop on PRs #319 and #322. Content re-applied on a fresh branch from current main. 28 tests passing.

Counts as 9th + 11th graduation (both originally stuck DIRTY).

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Copilot]

Pull request overview

Consolidates previously-stuck work from #319 and #322 by adding composable Graph operators (delegating to ZSet algebra) and implementing Newman modularity as a Graph metric, along with new unit tests.

Changes:

• Add Graph.map, Graph.filter, Graph.distinct, Graph.union, and Graph.difference as thin wrappers over ZSet operators.
• Add Graph.modularityScore (Newman modularity on symmetrized adjacency) returning double option.
• Extend Graph.Tests.fs with coverage for the new operators and modularity behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/Core/Graph.fs	Adds ZSet-backed operator composition helpers and implements modularityScore.
tests/Tests.FSharp/Algebra/Graph.Tests.fs	Adds tests validating operator semantics and modularity scoring on small graphs.

[Copilot]

modularityScore assigns missing nodes to community ids -(i + 1). Because callers can legally use negative community labels in partition, this can collide and incorrectly treat unrelated nodes as being in the same community, contradicting the doc comment that missing nodes become singleton groups with unique labels. Consider generating fresh ids that are guaranteed not to overlap with any existing label (e.g., allocate from minLabel-1 downward, or map communities to an internal discriminated union like Known of int | Missing of int).

Suggested change

	| Some c -> c
	| None -> -(i + 1)
	| Some c -> true, c
	| None -> false, i

[Copilot]

The doc comment claims modularityScore returns Q in [-0.5, 1], but Graph explicitly supports negative edge weights (anti-edges) and this implementation incorporates signed weights into A_sym/degrees. With signed weights, modularity is not generally bounded in that range. Either restrict/normalize the input to non-negative weights (and document/validate that), or relax the stated range to avoid misleading callers.

Suggested change

	/// in [-0.5, 1]; None for empty graph or zero total weight.
	/// for non-empty graphs with non-zero total symmetrized
	/// weight; None for empty graphs or when the total
	/// symmetrized weight is zero. Because `Graph` permits
	/// signed edge weights and this computation incorporates
	/// those signed weights directly, no fixed numeric bound
	/// is guaranteed for the returned score.

[Copilot]

modularityScore recomputes community i/community j (including a Map.tryFind) inside the nested for i/for j loops, which adds avoidable overhead on an already O(n²) routine. Precompute a communities: int[] once (and possibly a lookup from community id to node indices) before the double loop to keep the inner loop allocation/lookup-free.

Suggested change

	let community i =
	let node = nodeList.[i]
	match Map.tryFind node partition with
	| Some c -> c
	| None -> -(i + 1)
	let mutable q = 0.0
	for i in 0 .. n - 1 do
	for j in 0 .. n - 1 do
	if community i = community j then
	let communities =
	Array.init n (fun i ->
	let node = nodeList.[i]
	match Map.tryFind node partition with
	| Some c -> c
	| None -> -(i + 1))
	let mutable q = 0.0
	for i in 0 .. n - 1 do
	for j in 0 .. n - 1 do
	if communities.[i] = communities.[j] then