taboege/graph-ci

Compute separation in undirected graphs and DAGs

graph-ci's Issues

Variable strings do not use `set` parameter

In this code, @M is created from symbols of 1..$n

graph-ci/lib/Cube/Variables.pm

Lines 65 to 70 in 651e7e6

	my @M = grep { $_ != $i and $_ != $j } 1 .. $n;
	for my $k (0 .. @M) {
	for my $L (reverse subsets([@M], $k)) {
	my $face = [$i,$j,sort @$L];
	$faces[$v] = $face;
	$names[$v] = "$i$j|" . join('', @$L);

but its subsets are then used to generate names of variables. This ignores the set parameter to the constructor which tells which symbol to use for which element of the ground set.

moralize needs support for isolated vertices

The graph packages don't support isolated vertices. This makes ->moralize return a wrong graph when the invocant is disconnected:

Graph::Directed->new(qw(12 23 45))->moralize(1,2,4)->vertices #= (1, 2)

The vertex 4 is isolated in the ancestral graph and gets lost completely in the undirected moral graph. I think connected graphs and the ->ci method in general are unaffected, but still this could be troublesome.

Variable ordering and reverse subsets

I think I recently discovered that the reverse in

graph-ci/lib/Cube/Variables.pm

Line 67 in 651e7e6

for my $L (reverse subsets([@M], $k)) {

is wrong. The correct ordering is produced without reverse. I do not know why it is there or if the implementation of that module changed...

I wonder what computation I needed this code for and if the result was wrong because of this.

More sorts required

The conditioning sets in

graph-ci/lib/Cube/Variables.pm

Line 70 in 651e7e6

$names[$v] = "$i$j|" . join('', @$L);

graph-ci/lib/Cube/Variables.pm

Line 116 in 651e7e6

my $ijK = join '', @{+shift};

should be sorted. Otherwise lookup may fail. This is probably not needed in the first case, but the second case has user-controlled input.