language icon indicating copy to clipboard operation
language copied to clipboard

feat: assign weights in graph

Open miparnisari opened this issue 1 year ago • 3 comments

Description

This is the scaffolding code required to compute weights for each node and edge. It's not meant to support every kind of model - more followup PRs will come. (For example, models with cycles are not handled).

Please review each model in the weighted_graph_builder_test.go and visualize the output in https://dreampuf.github.io/GraphvizOnline/. This will help you review the correctness of the weight assignment algorithm.

Example

image

miparnisari avatar Sep 18 '24 05:09 miparnisari

Why not use gonum's weighted graph? 🤔

elbuo8 avatar Sep 23 '24 16:09 elbuo8

Why not use gonum's weighted graph? 🤔

@elbuo8 several reasons:

  1. gonum's weighted graph allows natively assigning weights to edges only
  2. those weights are float64 only: https://pkg.go.dev/gonum.org/v1/gonum/graph#WeightedLine

miparnisari avatar Sep 23 '24 19:09 miparnisari

Non-Blocking

The relationship between the weighted graph builder and the weighted graph is a bit odd. Currently the weighted graph has a dependency of its builder, which seems backwards.

Instead of coupling it all together with the current approach:

weightedGraph, err := NewWeightedAuthorizationModelGraph(model)

I would suggest decoupling them explicitly:

weightedGraphBuilder := NewWeightedAuthorizationModelGraphBuilder()
weightedGraph, err := weightedGraphBuilder.Build(model)

I know this is introducing an additional step/line of code, however I really don't see the point of having the builder, otherwise.

senojj avatar Sep 25 '24 17:09 senojj