Skip to content

Implement outneighbors and inneighbors - #34

Closed
singhharsh1708 wants to merge 1 commit into
JuliaGraphs:masterfrom
singhharsh1708:feat/neighbors
Closed

singhharsh1708 wants to merge 1 commit into
JuliaGraphs:masterfrom
singhharsh1708:feat/neighbors

Conversation

@singhharsh1708

@singhharsh1708 singhharsh1708 commented Sep 7, 2026 •

Copy link
Copy Markdown

Fills in the Graphs.outneighbors and Graphs.inneighbors TODOs. degree, connected_components, gdistances and is_connected all work on a VNGraph after this and agree with the SimpleGraph conversion, where before they errored.

very_nauty stores each adjacency list in insertion order, not sorted, so this returns a fresh sorted Vector{Cuint} rather than exposing the C array. VNGraph is an AbstractSimpleGraph, and the inherited methods assume sorted adjacency: SimpleEdgeIter calls searchsortedfirst(fadj(g, u), u) to skip the lower triangle. Defining fadj over the raw array would make edges and issubset return wrong answers instead of erroring.

A self loop is stored twice in the vertex's own adjacency list where SimpleGraph lists it once, so the repeat is collapsed after sorting. The C adjacency is read under GC.@preserve g, since the raw pointer is held across the allocation of the result. An out-of-range vertex would be an out-of-bounds read on a C pointer, so it throws BoundsError, matching SimpleGraph.

Verified against the SimpleGraph conversion over every vertex of 20 random regular graphs, plus sortedness, element type, the empty neighbourhood, out-of-range, a self loop, and shuffled insertion order: 703 new checks, 815 passing. Reverting src/VNGraphs.jl turns those 703 into 6 failures and 697 errors.

Part of JuliaGraphs/Graphs.jl#448. #35 builds on this one; the rest are independent.

AI Disclosure: Claude assisted with the code. Testing and review by @singhharsh1708.

@codecov

codecov Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.56%. Comparing base (0fa35cb) to head (58dc959).

Additional details and impacted files
@@             Coverage Diff             @@
##           master      #34       +/-   ##
===========================================
+ Coverage   45.90%   60.56%   +14.66%     
===========================================
  Files           1        1               
  Lines          61       71       +10     
===========================================
+ Hits           28       43       +15     
+ Misses         33       28        -5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@singhharsh1708

Copy link
Copy Markdown
Author

One design question I should raise rather than decide. Graphs.jl 1.14 made neighbors, inneighbors and outneighbors return an immutable FrozenVector instead of a Vector, and on 1.15 outneighbors(::SimpleGraph, v) returns Graphs.FrozenVector{Int64} while this returns a plain Vector{Cuint}. Returning a mutable vector is safe here, since it is freshly allocated on every call and I checked that mutating it leaves the graph untouched, but it does not match what SimpleGraph now guarantees to callers. Wrapping it would mean raising the Graphs compat bound from 1.12, which felt like your call rather than something to fold into this PR.

@singhharsh1708

singhharsh1708 commented Sep 7, 2026 •

Copy link
Copy Markdown
Author

For the record, #14 defines outneighbors and inneighbors as well. The difference is sorting: it returns the C adjacency in stored order, which is insertion order in very_nauty, while the inherited SimpleEdgeIter and issubset assume sorted lists. This sorts, collapses the doubled self-loop entry, and throws BoundsError out of range where #14 returns an empty vector.

@Krastanov

Copy link
Copy Markdown
Member

LLM work from new contributors is not acceptable for bounties

@Krastanov Krastanov closed this Sep 12, 2026
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.

2 participants