Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transitive reduction is incorrect #91

Closed
benedictleejh opened this issue Dec 23, 2019 · 3 comments
Closed

Transitive reduction is incorrect #91

benedictleejh opened this issue Dec 23, 2019 · 3 comments

Comments

@benedictleejh
Copy link
Contributor

There seems to be a bug in the transitive reduction computation.

Given the following graph:

a - b - c - d - e
    \__________/

The expected transitive reduction is:

a - b - c - d - e

However, running the transitive_reduction function on it results in an unchanged graph.

The code I used for the above graph is:

open Graph.Pack.Digraph
let g1 = create ()
let v1 = V.create 1
let v2 = V.create 2
let v3 = V.create 3
let v4 = V.create 4
let v5 = V.create 5
let () = add_vertex g1 v1
let () = add_vertex g1 v2
let () = add_vertex g1 v3
let () = add_vertex g1 v4
let () = add_vertex g1 v5
let () = add_edge g1 v1 v2
let () = add_edge g1 v2 v3
let () = add_edge g1 v3 v4
let () = add_edge g1 v4 v5
let () = add_edge g1 v2 v5

I'm also attaching the dot output for the initial graph, and the supposed transitively reduced graph. The tred program in graphviz computes the expected transitive reduction.

raw-dot.txt
reduced-dot.txt

@backtracking
Copy link
Owner

Indeed the reduction is not minimal on this example (though we have the expected property that the transitive closure of the output is the same as the transitive closure of the input).
I agree that, on DAGs at least, the reduction could be made minimal (and unique), with some extra effort.

@backtracking
Copy link
Owner

I tentatively fixed transitive_reduction in commit 69fd491.
Please have a look and let me know if this is fine with you.

@benedictleejh
Copy link
Contributor Author

I ran some quick and dirty tests on my end, and the fixed transitive_reduction performs as expected. It looks fine to me, so feel free to close this issue. (I could close it, but maybe you'd like to keep it around until you have more comprehensive tests for the fix?)

backtracking added a commit to backtracking/opam-repository that referenced this issue Oct 2, 2020
CHANGES:

- port to dune and opam 2.0
  - ❗ opam package now split into two packages: ocamlgraph and ocamlgraph_gtk
  - [WeakTopological] fixed incorrect use of generic hash tables
    (backtracking/ocamlgraph#99, Tomáš Dacík)
  - [Oper] fixed transitive_reduction (backtracking/ocamlgraph#91)
  - fix incorrect uses of polymorphic equality (Steffen Smolka, Boris Yakobowski)
  - [Coloring] fixed generation of OCamlDoc documentation
    (contributed by Earnestly)
  - ❗ [Coloring] functions now fail if the graph is directed
  - ❗ [Coloring] now uses a single, global exception [NoColoring]
  - [Coloring] new function two_color to 2-color a graph (or fail)
  - ❗ [Fixpoint] Take initial labeling of nodes into account (Johannes Kloos)
mseri pushed a commit to ocaml/opam-repository that referenced this issue Oct 8, 2020
* [new release] ocamlgraph_gtk and ocamlgraph (2.0.0)

CHANGES:

- port to dune and opam 2.0
  - ❗ opam package now split into two packages: ocamlgraph and ocamlgraph_gtk
  - [WeakTopological] fixed incorrect use of generic hash tables
    (backtracking/ocamlgraph#99, Tomáš Dacík)
  - [Oper] fixed transitive_reduction (backtracking/ocamlgraph#91)
  - fix incorrect uses of polymorphic equality (Steffen Smolka, Boris Yakobowski)
  - [Coloring] fixed generation of OCamlDoc documentation
    (contributed by Earnestly)
  - ❗ [Coloring] functions now fail if the graph is directed
  - ❗ [Coloring] now uses a single, global exception [NoColoring]
  - [Coloring] new function two_color to 2-color a graph (or fail)
  - ❗ [Fixpoint] Take initial labeling of nodes into account (Johannes Kloos)

* ocamlgraph.2.0.0: added depends graphics with-test

* ocamlgraph_gtk.2.0.0: added depends graphics with-test

* ocamlgraph 2.0.0 requires OCaml >= 4.03.0

* added a constraint 'ocamlgraph <= 1.8.8'

* better constraints (suggested by David Allsopp)
fdopen added a commit to fdopen/opam-repository-mingw that referenced this issue Oct 8, 2020
* [new release] ocamlgraph_gtk and ocamlgraph (2.0.0)

CHANGES:

- port to dune and opam 2.0
  - ❗ opam package now split into two packages: ocamlgraph and ocamlgraph_gtk
  - [WeakTopological] fixed incorrect use of generic hash tables
    (backtracking/ocamlgraph#99, Tomáš Dacík)
  - [Oper] fixed transitive_reduction (backtracking/ocamlgraph#91)
  - fix incorrect uses of polymorphic equality (Steffen Smolka, Boris Yakobowski)
  - [Coloring] fixed generation of OCamlDoc documentation
    (contributed by Earnestly)
  - ❗ [Coloring] functions now fail if the graph is directed
  - ❗ [Coloring] now uses a single, global exception [NoColoring]
  - [Coloring] new function two_color to 2-color a graph (or fail)
  - ❗ [Fixpoint] Take initial labeling of nodes into account (Johannes Kloos)

* ocamlgraph.2.0.0: added depends graphics with-test

* ocamlgraph_gtk.2.0.0: added depends graphics with-test

* ocamlgraph 2.0.0 requires OCaml >= 4.03.0

* added a constraint 'ocamlgraph <= 1.8.8'

* better constraints (suggested by David Allsopp)
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

No branches or pull requests

2 participants