Floyd-Warshall #70

lukem12345 · 2024-08-24T06:28:52Z

The Floyd-Warshall algorithm can be defined on the variables of a Decapode. This entails a topological ordering of those variables. A topological ordering of operations is given in turn.

Added `apex` and `@relation`, `to_graphviz` from Catlab Co-authored-by: James <[email protected]>

Converts ACSet to a series of Symbolic terms that can be rewritten with a provided rewriter

codecov · 2024-08-24T06:33:08Z

Codecov Report

Attention: Patch coverage is 67.74194% with 30 lines in your changes missing coverage. Please review.

Project coverage is 82.49%. Comparing base (2a6269c) to head (a93295a).

Files with missing lines	Patch %	Lines
src/graph_interface.jl	64.06%	23 Missing ⚠️
src/acset2symbolic.jl	75.86%	7 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff                @@
##           gr/acset2sym      #70      +/-   ##
================================================
- Coverage         84.37%   82.49%   -1.89%     
================================================
  Files                15       15              
  Lines               960     1011      +51     
================================================
+ Hits                810      834      +24     
- Misses              150      177      +27

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

lukem12345 · 2024-08-24T16:35:53Z

Commit d46cf86 added both docstrings and introduced a multiple dispatch scheme by subtyping TraversalNode. The MD scheme can be undone depending on design constraints I’m not aware of. This results in cleaner algorithm code

lukem12345 · 2024-08-24T19:12:06Z

It would be a fun project to investigate whether and how this can be framed in terms of an “Algebraic Path Problem”.[1,2]

[1] https://drops.dagstuhl.de/storage/00lipics/lipics-vol211-calco2021/LIPIcs.CALCO.2021.20/LIPIcs.CALCO.2021.20.pdf
[2] https://edge.edx.org/asset-v1:BerkeleyX+CS188-SU16+SU16+type@asset+block/kleene_algebras_path_problems.pdf

GeorgeR227 · 2024-08-26T15:46:56Z

src/graph_traversal.jl

+function topological_sort_verts(d::SummationDecapode)
+  m = floyd_warshall(d)
+  map(parts(d,:Var)) do v
+    minimum(filter(!isinf, m[v,infer_terminals(d)]))
+  end
 end


Don't do Floyd-Warshall just to do a topological sort. F-W is a more complicated algorithm, has a much higher time complexity, and I'm not entirely convinced that this gets us what we want. Leave the previous implementation of topological sort, since that has been well vetted, and if you want, use the topological sorting to run a shortest paths algorithm.

Return a list of TraversalNode for :Var for the output.

GeorgeR227 · 2024-08-26T15:47:40Z

test/graph_traversal.jl

+  # XXX Do cycle-detection with FW by using ∞ on the diagonal.
+  #=
  cyclic = @decapode begin
    B == g(A)
    A == f(B)
  end
  @test_throws AssertionError topological_sort_edges(cyclic)
+  =#


Why is this test removed? I don't think we do cycle detection anywhere else in the parsing pipeline so this should be checked here.

GeorgeR227 · 2024-08-26T15:48:10Z

test/graph_traversal.jl

+
+  sum_with_single_dependency = @decapode begin
+    F == A + f(A) + h(g(A))
+  end
+  result = topological_sort_edges(sum_with_single_dependency)
+  @test is_correct_length(sum_with_single_dependency, result)


Add tests for the F-W algorithm.

GeorgeR227 · 2024-08-26T16:07:10Z

src/graph_traversal.jl

+  dom::AbstractVector
+  cod::Int


Call these src/tgt since we're talking about hypergraphs.

GeorgeR227 · 2024-08-26T17:05:00Z

src/graph_traversal.jl

+TraversalNode(i, d::SummationDecapode, ::Val{:Op1}) =
+  TraversalNode{Symbol}(i, d[i,:op1], [d[i,:src]], d[i,:tgt])

-  # TODO: Collect these visited arrays into one structure indexed by :Op1, :Op2, and :Σ
-  visited_1 = falses(nparts(d, :Op1))
-  visited_2 = falses(nparts(d, :Op2))
-  visited_Σ = falses(nparts(d, :Σ))
+TraversalNode(i, d::SummationDecapode, ::Val{:Op2}) =
+  TraversalNode{Symbol}(i, d[i,:op2], [d[i,:proj1],d[i,:proj2]], d[i,:res])

-  # FIXME: this is a quadratic implementation of topological_sort inlined in here.
-  op_order = TraversalNode{Symbol}[]
+TraversalNode(i, d::SummationDecapode, ::Val{:Σ}) =
+  TraversalNode{Symbol}(i, :+, d[incident(d,i,:summation),:summand], d[i,:sum])


Instead of the dispatching being a seperate argument, maybe we can bake that dispatching into the struct type, so we have TraversalNode{Op1} instead of TraversalNode{Symbol} with a name field that is :Op1.

GeorgeR227 · 2024-08-26T17:07:20Z

src/graph_traversal.jl

+function hyper_edge_list(d::SummationDecapode)
+  [map(e -> TraversalNode(e, d, Val(:Op1)), parts(d, :Op1))...,
+   map(e -> TraversalNode(e, d, Val(:Op2)), parts(d, :Op2))...,
+   map(e -> TraversalNode(e, d, Val(:Σ  )), parts(d, :Σ  ))...]
+end


Have a function that returns all of the "edge" symbols, so in this case Op1, Op2, Σ and just iterate over those with the interior functionality. Can also do this with all the "vertex" symbols.

lukem12345 · 2024-08-26T19:26:29Z

@GeorgeR227 The overarching purpose of this PR to demonstrate how to transform Decapode into a well-known data structure, i.e. a graph, at which point we can use generic traversal algorithms. Not bespoke or manually-inlined traversal algorithms over the bespoke SummationDecapode data structure. To wit, compare the Floyd-Warshall algorithm as implemented here with the pseudocode given for it on Wikipedia. The choice of Floyd-Warshall vs. multi-start Dijkstra's vs. Kuhn's and so on is somewhat beside the point, but of course time and space complexity, weighed against the average size of our datastructures (< 1000 vertices), are to be weighed against maintainability before such a PR is merged. Treat it as a proof of concept for the time being.

GeorgeR227 · 2024-08-26T20:54:13Z

I get your idea about make our hypergraph algorithms more generic. I think many people have wanted that and I do want to keep those parts of this PR.

What I don't get is the decision to base everything on Floyd-Warshall. If F-W can be made into a generic hypergraph algorithm, then why can't topological sort? The implementation was almost there, it just needed the concept of the generic edge (which you implemented here) plus a bit of other functionality probably.

My main idea here is that the decision to use F-W makes this code less scalable/performant, less readable (since this is no longer the standard topological sort algorithm most people are familiar with) and less maintainable (because now we have graph algorithms which depend on other graph algorithms when there's no reason they have to).

I like the push towards going generic (even though that wasn't the explicit push of the branch this is based off) and I don't even mind the inclusion of F-W but it should not be used in the topological sort.

lukem12345 · 2024-08-26T21:52:19Z

We're on the same page

This struct organizes the data into a more generic hypergraph that can then be routed through generic graph algorithms, like topo sort or F-W, without relying on the underlying ACSet structure.

lukem12345 · 2024-08-28T19:20:34Z

@GeorgeR227 The most recent commit needs to use git mv. We shouldn't delete files and place their contents in new files.

GeorgeR227 · 2024-08-28T20:20:06Z

Yeah I can do that. Any thoughts on the generic HyperGraph?

lukem12345 · 2024-08-29T01:52:21Z

We should probably just switch to using an incidence matrix. Also accommodates arguments of arbitrary arity. In Decapodes then just "dispatch"/ switch/ pattern-match on arity and name of symbol. Restricting operations to unary ops, binary ops, and sums is no longer is a necessary design constraint.

lukem12345 · 2024-08-29T02:39:45Z

Or just use this schema, migration or otherwise. Nodes are rows in the Var table. Edges are cached calls to

(dom = d[incident(d,i,:arg_op),:arg_var], cod = d[i,:res], op = function[i])

@schema SchVariadicDecapode begin # SchHyperGraph
  Var::Ob
  (Type, VarName, Operator)::AttrType(Symbol)
  type::Attr(Var, Type)
  name::Attr(Var, Name)

  (Op, Argument)::Ob
  (Function)::AttrType(Symbol)
  function::Attr(Op, Function)
  arg_var::Hom(Argument, Var)
  arg_op::Hom(Argument, Op)
  res::Hom(Op, Var)
end

algebraicjuliabot and others added 7 commits August 20, 2024 16:00

Set version to 0.1.7

d2fa4a4

Added more exports (#44)

b14f09d

Added `apex` and `@relation`, `to_graphviz` from Catlab Co-authored-by: James <[email protected]>

Add type rules for vectorfields

c9b8898

Add musical overload resolution

79f27e7

Take advantage of :infer in type rules

23fbf3f

Initial attempt at rewriting

2a6269c

Converts ACSet to a series of Symbolic terms that can be rewritten with a provided rewriter

Define Floyd-Warshall algorithm on Decapodes

2512181

lukem12345 requested review from jpfairbanks and GeorgeR227 August 24, 2024 06:28

Remove unnecessary DataStructures dependency

31dc670

Add docstrings and use multiple dispatch

d46cf86

lukem12345 and others added 3 commits August 24, 2024 14:12

Explicitly create hyperedge list

db988a2

Clean by caching hyperedge order

0b52472

Fix comment typo cod -> dom

7daf15b

Compute longest paths from terminals

303962d

GeorgeR227 requested changes Aug 26, 2024

View reviewed changes

Just use -1 as weight

dfe0e03

lukem12345 mentioned this pull request Aug 27, 2024

Use traversal abstraction AlgebraicJulia/Decapodes.jl#270

Open

GeorgeR227 added 2 commits August 28, 2024 14:24

Addition of generic graph struct

db2274f

This struct organizes the data into a more generic hypergraph that can then be routed through generic graph algorithms, like topo sort or F-W, without relying on the underlying ACSet structure.

Remove old graph_traversal file

a93295a

GeorgeR227 force-pushed the gr/acset2sym branch from 2a6269c to 9d8dfca Compare August 30, 2024 18:33

GeorgeR227 force-pushed the gr/acset2sym branch 2 times, most recently from 378d6a1 to e0ff9a8 Compare September 18, 2024 17:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Floyd-Warshall #70

Floyd-Warshall #70

lukem12345 commented Aug 24, 2024

codecov bot commented Aug 24, 2024 •

edited

Loading

lukem12345 commented Aug 24, 2024

lukem12345 commented Aug 24, 2024

GeorgeR227 Aug 26, 2024

GeorgeR227 Aug 26, 2024

GeorgeR227 Aug 26, 2024

GeorgeR227 Aug 26, 2024

GeorgeR227 Aug 26, 2024

GeorgeR227 Aug 26, 2024

GeorgeR227 Aug 26, 2024

lukem12345 commented Aug 26, 2024

GeorgeR227 commented Aug 26, 2024

lukem12345 commented Aug 26, 2024

lukem12345 commented Aug 28, 2024

GeorgeR227 commented Aug 28, 2024

lukem12345 commented Aug 29, 2024 •

edited

Loading

lukem12345 commented Aug 29, 2024 •

edited

Loading

Floyd-Warshall #70

Are you sure you want to change the base?

Floyd-Warshall #70

Conversation

lukem12345 commented Aug 24, 2024

codecov bot commented Aug 24, 2024 • edited Loading

Codecov Report

lukem12345 commented Aug 24, 2024

lukem12345 commented Aug 24, 2024

GeorgeR227 Aug 26, 2024

Choose a reason for hiding this comment

GeorgeR227 Aug 26, 2024

Choose a reason for hiding this comment

GeorgeR227 Aug 26, 2024

Choose a reason for hiding this comment

GeorgeR227 Aug 26, 2024

Choose a reason for hiding this comment

GeorgeR227 Aug 26, 2024

Choose a reason for hiding this comment

GeorgeR227 Aug 26, 2024

Choose a reason for hiding this comment

GeorgeR227 Aug 26, 2024

Choose a reason for hiding this comment

lukem12345 commented Aug 26, 2024

GeorgeR227 commented Aug 26, 2024

lukem12345 commented Aug 26, 2024

lukem12345 commented Aug 28, 2024

GeorgeR227 commented Aug 28, 2024

lukem12345 commented Aug 29, 2024 • edited Loading

lukem12345 commented Aug 29, 2024 • edited Loading

codecov bot commented Aug 24, 2024 •

edited

Loading

lukem12345 commented Aug 29, 2024 •

edited

Loading

lukem12345 commented Aug 29, 2024 •

edited

Loading