Commit 8df5f513 authored by Alexandre Duret-Lutz's avatar Alexandre Duret-Lutz

gfguarantee: fix #357 again

The previous patch triggered this issue again, failing
core/ltl2tgba2.test.

* spot/twaalgos/gfguarantee.cc: Separate the replaying of history from
the modification of the automaton.
* NEWS: Mention the bug.
* tests/python/twagraph-internals.ipynb, tests/python/automata.ipynb:
Adjust.
parent da5d23f0
......@@ -132,6 +132,12 @@ New in spot 2.7.5.dev (not yet released)
properties. This can be altered with the SPOT_PR_CHECK
environment variable.
Bugs fixed:
- The gf_guarantee_to_ba() is relying on an inplace algorithm that
could produce a different number of edges for the same input in
two different transition order.
New in spot 2.7.5 (2019-06-05)
Build:
......
......@@ -185,34 +185,29 @@ namespace spot
// }
}
unsigned nedges = aut->num_edges();
unsigned new_init = -1U;
// The loop might add new edges, but we just want to iterate
// on the initial ones.
for (unsigned edge = 1; edge <= nedges; ++edge)
// We do two the rewrite in two passes. The first one
// replays histories to detect the new source the edges
// should synchronize with. We used to have single
// loop, but replaying history on edges that have been modified
// result in different automaton depending on the edge order.
std::vector<unsigned> redirect_src;
if (is_det)
{
redirect_src.resize(aut->edge_vector().size());
for (auto& e: aut->edges())
{
auto& e = aut->edge_storage(edge);
unsigned edge = aut->edge_number(e);
redirect_src[edge] = e.src; // no change by default
// Don't bother with terminal states, they won't be
// reachable anymore.
if (term[si.scc_of(e.src)])
continue;
// It will loop
if (term[si.scc_of(e.dst)])
{
// If the source state is the initial state, we
// should not try to combine it with itself...
//
// Also if the automaton is not deterministic
// and there is no trivial initial state, then
// we simply loop back to the initial state.
if (e.src == init
|| (!is_det && !initial_state_trivial))
{
e.dst = init;
e.acc = {0};
new_init = init;
// It will not loop
if (!term[si.scc_of(e.dst)])
continue;
}
// It will loop.
// If initial state cannot be reached from another
// state of the automaton, we can get rid of it by
// combining the edge reaching the terminal state
......@@ -232,9 +227,7 @@ namespace spot
// cases, we should try again with a smaller history.
unsigned moved_init = init;
bdd econd = e.cond;
bool first;
if (is_det)
{
auto& h = histories[e.src];
int hsize = h.size();
for (int hlen = hsize - 1; hlen >= 0; --hlen)
......@@ -266,11 +259,64 @@ namespace spot
&& term[si.scc_of(ei.dst)])
goto failed;
// we have successfully played all history
// to a non-terminal state.
redirect_src[edge] = moved_init;
break;
failed:
moved_init = init;
continue;
}
}
}
// No we do the redirection.
for (auto& e: aut->edges())
{
// Don't bother with terminal states, they won't be
// reachable anymore.
if (term[si.scc_of(e.src)])
continue;
// It will loop
if (term[si.scc_of(e.dst)])
{
// If the source state is the initial state, we
// should not try to combine it with itself...
//
// Combine this edge with any compatible
// edge from the initial state.
// Also if the automaton is not deterministic
// and there is no trivial initial state, then
// we simply loop back to the initial state.
if (e.src == init
|| (!is_det && !initial_state_trivial))
{
e.dst = init;
e.acc = {0};
new_init = init;
continue;
}
// If initial state cannot be reached from another
// state of the automaton, we can get rid of it by
// combining the edge reaching the terminal state
// with the edges leaving the initial state.
//
// However if we have some histories for e.src
// (which implies that the automaton is
// deterministic), we can try to replay that from
// the initial state first.
//
// One problem with those histories, is that we do
// not know how much of it to replay. It's possible
// that we cannot find a matching transition (e.g. if
// the history if "b" but the choices are "!a" or "a"),
// and its also possible to that playing too much of
// history will get us back the terminal state. In both
// cases, we should try again with a smaller history.
unsigned moved_init = init;
bdd econd = e.cond;
bool first;
if (is_det)
{
unsigned edge = aut->edge_number(e);
moved_init = redirect_src[edge];
first = true;
for (auto& ei: aut->out(moved_init))
{
......@@ -285,7 +331,6 @@ namespace spot
bool ts = term[si.scc_of(ei.dst)];
if (ts)
{
assert(hlen == 0);
want_merge_edges = true;
}
if (first)
......@@ -317,13 +362,7 @@ namespace spot
cond, {0});
}
}
}
}
break;
failed:
moved_init = init;
continue;
}
}
else
......
This diff is collapsed.
This diff is collapsed.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment