Dijkstra Path generic, Closeness Centrality, Iterate Edge with directionality

[dizzyi]

Development Workflow

• make format
• make test
• make lint

error: very complex type used. Consider factoring parts into `type` definitions
   --> src\core\paths.rs:148:6
    |
148 | ) -> Result<(Vec<Option<f64>>, Vec<Option<NodeId>>), GraphinaException>
    |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
    = note: `-D clippy::type-complexity` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::type_complexity)]`

error: very complex type used. Consider factoring parts into `type` definitions
   --> src\core\paths.rs:246:6
    |
246 | ) -> Result<(Vec<Option<f64>>, Vec<Option<NodeId>>), GraphinaException>
    |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity

error: could not compile `graphina` (lib) due to 2 previous errors
make: *** [Makefile:84: lint] Error 101

dijkstra_path_impl

this function is basically a copy of dijkstra, I didn't change dijkstra since it is used heavily in other part of the codebase, you can decide how to refactor later.

what's difference

• take cutoff for maximum cost for path when pathfinding, inline with networkx's api
• take eval_cost for evaluating cost base on given edge type
  • impl Fn(&W) -> Option<f64> type callback
  • return Some(f64) for edge's cost
  • return None for impassable edge
• return ok with(Vec<Option<f64>>, Vec<Option<NodeId>>)
  • first term for reach cost
  • second term for path backtrack
• internally use ordered_float::NotNan for BinaryHeap, instead of making user use OrderedFloat.

pub fn dijkstra_path_impl<A, W, Ty>(
    graph: &BaseGraph<A, W, Ty>,
    source: NodeId,
    cutoff: Option<f64>,
    eval_cost: impl Fn(&W) -> Option<f64>,
) -> Result<(Vec<Option<f64>>, Vec<Option<NodeId>>), GraphinaException>
where
    W: Debug,
    A: Debug,
    Ty: GraphConstructor<A, W>,
    NodeId: Ord,
    BaseGraph<A, W, Ty>: GraphinaGraph<A, W>,
{ /* ... */ }

cleaner interface for edge type f64

pub fn dijkstra_path_f64<A, Ty>(
    graph: &BaseGraph<A, f64, Ty>,
    source: NodeId,
    cutoff: Option<f64>,
) -> Result<(Vec<Option<f64>>, Vec<Option<NodeId>>), GraphinaException>
where
    A: Debug,
    Ty: GraphConstructor<A, f64>,
    BaseGraph<A, f64, Ty>: GraphinaGraph<A, f64> 
{ /* ... */}

Example

use graphina::core::types::Digraph;

use graphina::core::paths::dijkstra_path_impl;

let mut graph: Digraph<String, (f64, String)> = Digraph::new();
//                             ^^^^^^^^^^^^^
//                                         L arbitrary type as edge

let cities = ["ATL", "PEK", "LHR", "HND", "CDG", "FRA", "HKG"];

let ids = cities
    .iter()
    .map(|s| graph.add_node(s.to_string()))
    .collect::<Vec<_>>();

let edges = [
    //
    ("ATL", "PEK", (900.0, "boeing")),
    ("ATL", "LHR", (500.0, "airbus")),
    ("ATL", "HND", (700.0, "airbus")),
    //
    ("PEK", "LHR", (800.0, "boeing")),
    ("PEK", "HND", (100.0, "airbus")),
    ("PEK", "HKG", (100.0, "airbus")),
    //
    ("LHR", "CDG", (100.0, "airbus")),
    ("LHR", "FRA", (200.0, "boeing")),
    ("LHR", "HND", (600.0, "airbus")),
    //
    ("HND", "ATL", (700.0, "airbus")),
    ("HND", "FRA", (600.0, "airbus")),
    ("HND", "HKG", (100.0, "airbus")),
    //
];

for (s, d, w) in edges {
    let depart = cities.iter().position(|city| s == *city).unwrap();
    let destin = cities.iter().position(|city| d == *city).unwrap();
    graph.add_edge(ids[depart], ids[destin], (w.0, w.1.to_string()));
}

// function for evaluating possible cost for the edge
// Some(f64) for cost
// None for impassable
let eval_cost = |(price, manufactuer): &(f64, String)| match manufactuer.as_str() {
    "boeing" => None,  // avoid boeing plane
    _ => Some(*price), // return price as the cost
};

let (cost, trace) = dijkstra_path_impl(&graph, ids[0], Some(1000.0), eval_cost).unwrap();

println!("cost : {:?}", cost);
println!("trace: {:?}", trace);
// cost : [Some(0.0), None, Some(500.0), Some(700.0), Some(600.0), None, Some(800.0)]
// trace: [None, None, Some(NodeId(NodeIndex(0))), Some(NodeId(NodeIndex(0))), Some(NodeId(NodeIndex(2))), None, Some(NodeId(NodeIndex(3)))]

Closeness Centrality

pub fn closeness_centrality_impl<A, W, Ty>(
    graph: &BaseGraph<A, W, Ty>,
    eval_cost: impl Fn(&W) -> Option<f64>,
    wf_improved: bool,
) -> Result<Vec<f64>, GraphinaException>
where
    A: Debug,
    W: Debug,
    Ty: GraphConstructor<A, W>,
    BaseGraph<A, W, Ty>: GraphinaGraph<A, W>,
{ /* ... */ }

pub fn closeness_centrality<A, Ty>(
    graph: &BaseGraph<A, f64, Ty>,
    wf_improved: bool,  // <========== api change, added wf improvement factor option, like networkx
) -> Result<Vec<f64>, GraphinaException>
where
    A: Debug,
    Ty: GraphConstructor<A, f64>,
    BaseGraph<A, f64, Ty>: GraphinaGraph<A, f64>,
{ /* ... */ }

GraphinaGraph trait

Why

since direction is accounted for in BaseGraph::edges(&self), when iterating through edge when direction matter, a check is needed.

previous

for (src, dst, w) in graph.edges() {
    let w = eval_weight(w);
    next[dst.index()] += w * centrality[src.index()];
    if !<Ty as GraphConstructor<A, W>>::is_directed() {
        next[src.index()] += w * centrality[dst.index()];
    }
}

now

doesn't need to check directionality when iterating edges.

flow_edges account for backward edge for undirected graph and just use forward edge for directed graph.

/// Extra util
pub trait GraphinaGraph<A, W> {
    /// return edges in form or `(src: NodeId, dst: NodeId, attr: &W)`,
    /// where the backward edge is included for undirected graph.
    fn flow_edges<'a>(&'a self) -> impl Iterator<Item = (NodeId, NodeId, &'a W)> + 'a
    where
        W: 'a;
}

for (src, dst, w) in graph.flow_edges() {
    let w = eval_weight(w);
    next[dst.index()] += w * centrality[src.index()];
}

Todo

Linter Error

the return type for dijkstra_path_impl is too complex for linter

Result<(Vec<Option<f64>>, Vec<Option<NodeId>>), GraphinaException>

maybe a custom result type

// crate::core::exceptions.rs
type Result<T> = std::result::Result<T, GraphinaException>;

crate::Result<(Vec<Option<f64>>, Vec<Option<NodeId>>)>

[habedi]

@dizzyi Thanks for the PR.
Can you make the linter checks pass? 😄

[dizzyi]

Okay, so I added this type alias.

pub type PathFindResult = (Vec<Option<f64>>, Vec<Option<NodeId>>);

idk what happens with the build test fail

[codecov]

Codecov Report

❌ Patch coverage is 70.31250% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.28%. Comparing base (f3cd4c5) to head (ac7b331).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/core/paths.rs	65.62%	11 Missing ⚠️
src/centrality/algorithms.rs	77.27%	5 Missing ⚠️
src/core/types.rs	70.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #20      +/-   ##
==========================================
- Coverage   80.57%   80.28%   -0.29%     
==========================================
  Files          12       12              
  Lines        1807     1882      +75     
==========================================
+ Hits         1456     1511      +55     
- Misses        351      371      +20     

☔ View full report in Codecov by Sentry.
📢 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.

[habedi]

Okay, so I added this type alias.

pub type PathFindResult = (Vec<Option<f64>>, Vec<Option<NodeId>>);

idk what happens with the build test fail

It was a problem on the GitHub side. I reran the failed test jobs, and they finished successfully.

[habedi]

LGTM 🚀