Hooray! :-)

I just merged the issue.

The change in fact_groups.py looks like a functional change that is good.

The changes in translate.py only affect functionality if we have a bug, in which case they replace a silent error with a loud one, which is good.

I tried doing a review but I don't understand the context well enough to say anything useful. I left a few very minor comments.

In case someone familiar with the translator wants to have a look: the only functional changes are in
  src/translate/fact_groups.py line 16
  (new filtering of unreachable facts in expand_group)
and in
  src/translate/translated.py lines 85, 114, and 628.
  (removed special case)

I ran experiments on the domains where the translator output changed:

Optimal search except M&S (ignore citycar and all unexplained error; the configurations do not support conditional effects):
https://ai.dmi.unibas.ch/_experiments/ai/downward/issue1079/data/issue1079-v3-search-opt-eval/issue1079-v3-search-opt-issue1079-base-issue1079-v3-compare.html

M&S: https://ai.dmi.unibas.ch/_experiments/ai/downward/issue1079/data/issue1079-v3-search-mas-eval/issue1079-v3-search-mas-issue1079-base-issue1079-v3-compare.html

Satisficing search: https://ai.dmi.unibas.ch/_experiments/ai/downward/issue1079/data/issue1079-v3-search-sat-eval/issue1079-v3-search-sat-issue1079-base-issue1079-v3-
compare.html

Scatter plots for total time are also available in the respective directories at https://ai.dmi.unibas.ch/_experiments/ai/downward/issue1079/data/.

I did not see anything that worries me in the results for optimal search w/o M&S. 

In the satisficing search, lama-first did not longer solve trucks-strips/p17 because it runs out of memory. I locally cannot reproduce that I get different output.sas files 
from the base and the v3 version, so this seems not to be related to change at hand but the remaining non-determinism in the invariant synthesis (from the frozenset iteration).

In M&S, we don't solve instance citycar-opt14-adl/p2-2-6-1-1.pddl anymore. This is a similar case. There are two encoding, both organizing non-binary variables by car. One 
covers predicates at_car_jun, at_car_road and starting, the other one also includes predicate arrived (in the first one represented in separate binary variables). In several 
runs, I saw in both versions both of the encodings, so this also does not seem to be related to our changes.

Does anyone else want to look at the experimental results? We also need a code review: https://github.com/aibasel/downward/pull/147

If someone wants to look at the changes: 
https://github.com/aibasel/downward/pull/147

Here are the experimental results:
- all: https://ai.dmi.unibas.ch/_experiments/ai/downward/issue1079/data/issue1079-v3-eval/issue1079-v3.html
- filtered: https://ai.dmi.unibas.ch/_experiments/ai/downward/issue1079/data/issue1079-v3-eval/issue1079-v3-
filtered.html

There are differences to the previous translator output in
- airport
- citycar
- freecell
- trucks-strip
- woodworking

In many tasks, the only difference is that we detect earlier that the final mutex group only contains an atom and its 
negation, and we only get a better human-readable key. E.g. from airport:

var0
-1
2
Atom is-moving(airplane_cfbeg)
NegatedAtom is-moving(airplane_cfbeg)

instead of 

var0
-1
2
Atom is-moving(airplane_cfbeg)
<none of those>

But in some tasks, we indeed get a different variable encoding (also in domains like woodworking-sat08, where the 
number of variables stays the same), so maybe we would like to have some experiments for these domains that also cover 
the search. What are your standard search configurations that are sensitive to the encoding?

I now remove all unreachable facts when instantiating mutex groups. Will start a 
new round of experiments...

I am going to have a look whether we can clean up the full mutex groups at an 
earlier stage.

The experiments have finished. At one place I had to revert the change because 
the full mutex groups could in contrast to our assumption contain facts that are 
not represented by a FDR variable.

With this change, we get the same output.sas files as before with the exception 
of some tasks in city-car and trucks-strips. I could confirm that in these 
domains we also can get different results with repeated runs of the unchanged 
Fast Downward (due to different orders of considering invariant parts in the 
invariant synthesis).

Full experimental results here: 
https://ai.dmi.unibas.ch/_experiments/ai/downward/issue1079/data/issue1079-v2-
eval/issue1079-v2.html
Filtered for tasks with different translator output: 
https://ai.dmi.unibas.ch/_experiments/ai/downward/issue1079/data/issue1079-v2-
eval/issue1079-v2-filtered.html

As a side effect of looking into issue1078 (going through the entire translator), 
we added typing information to some crucial classes in the translator. We also 
changed the logic at two places, where we had the impression that the code 
unnecessarily covers situations that never can happen. The next step will be to 
run an experiment to verify that everything still works.