Add distance metrics DigraphRadius, DigraphCentre, and DigraphPeriphery

[William-Kenyon]

Picks up on the work of PR #761; it is over a year old, so easier to start fresh. This implementation is simpler, handles the 0 vertex edge case, has more testing, and includes documentation.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.48%. Comparing base (8d49e9a) to head (80f100c).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #952      +/-   ##
==========================================
+ Coverage   97.45%   97.48%   +0.03%     
==========================================
  Files          50       53       +3     
  Lines       21189    21392     +203     
  Branches      639      639              
==========================================
+ Hits        20649    20854     +205     
+ Misses        475      473       -2     
  Partials       65       65              

☔ 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.

[mtorpey]

Great work! A few things to change:

• DigraphRadius probably shouldn't throw an error when given a digraph in which radius is not defined (not SC or no vertices). Returning fail would be more sensible.
• DigraphCentre should have a synonym called DigraphCenter (for the Americans out there).
• "A list of vertices" should probably be "A list of positive integers", since we want it basically to describe the type.
• DigraphDistanceMetrics probably shouldn't be user-facing, so call it DIGRAPHS_DistanceMetrics instead and don't include it directly in tests. Or if we really want it user-facing, add documentation.

[reiniscirpons]

@mtorpey wouldn't it make more sense to return infinity instead of fail for digraphs that are not strongly connected?

[mtorpey]

@reiniscirpons I think not. A vertex's eccentricity is "the greatest distance from it to any other vertex." To me, if you ask the distance between two unconnected vertices the answer shouldn't be "infinitely far" but "undefined".

As currently written, the doc only defines these terms for strongly connected digraphs.

[reiniscirpons]

@reiniscirpons I think not. A vertex's eccentricity is "the greatest distance from it to any other vertex." To me, if you ask the distance between two unconnected vertices the answer shouldn't be "infinitely far" but "undefined".

As currently written, the doc only defines these terms for strongly connected digraphs.

Yeah that makes sense, its also consistent with what shortest path function already in the library does.

Mathematically it is sometimes (see e.g. Wikipedia page on graph distance) taken as infinity to make some of the treatment more uniform e.g. when working with adjacency matrices and the like. But it essentially works the same as fail does for us, and should probably be handled separately anyway.

[james-d-mitchell]

This is good, thanks for the contribution @William-Kenyon, please update according to my comments and then I'll re-review.

[james-d-mitchell]

Suggested change

[james-d-mitchell]

This seems unnecessarily complicated to me (it's not very complicated, but nonetheless). I'd prefer this to be done as follows:

1. Implement a DigraphEccentricities attribute that returns List(DigraphShortestDistances(D), Maximum);
2. Implement DigraphRadius that is returns Minimum(DigraphEccentricities(D))
3. Implement DigraphCentre that implements Positions(DigraphEccentricities(D), DigraphRadius(D))
4. Implement DigraphPeriphery that returns Positions(DigraphEccentricities(D), DigraphDiameter(D))

basically returning a record containing three things that are more or less trivial to compute is just not necessary, and so to reduce the complexity (the complexity of the code not the implemented algorithm) as above is likely better.