Pushed to the master repository.

Thank you for your feedback! I fixed the typos and removed the left-over explicit.

I've taken a look and everything seems fine to me.
Looks that we agree in the interface for once.

I see. I'm done with the comments on bitbucket.

An "EOF" file error, which unfortunately sometimes happens on our grid.
Repeating the experiment usually resolves it.

Traceback (most recent call last):
  File
"/infai/sieverss/repos/downward/dev/experiments/issue655/data/issue655-base/code-216942f7ce83/builds/release32/bin/translate/translate.py",
line 22, in <module>
    import instantiate
  File
"/infai/sieverss/repos/downward/dev/experiments/issue655/data/issue655-base/code-216942f7ce83/builds/release32/bin/translate/instantiate.py",
line 8, in <module>
    import pddl_to_prolog
  File
"/infai/sieverss/repos/downward/dev/experiments/issue655/data/issue655-base/code-216942f7ce83/builds/release32/bin/translate/pddl_to_prolog.py",
line 7, in <module>
    import normalize
EOFError: EOF read where object expected

I saw a few things in the pull request; will write some comments.

Do you know what this "unexplained critical error" is about?

Results are fine:
http://ai.cs.unibas.ch/_tmp_files/sieverss/issue655-base-v1-issue655-base-issue655-v1-compare.html

Does anyone still wants to review the pull request? Otherwise, in my opinion,
this is ready to be merged.

The documentation of shrink strategies now mentions "max_states" etc as
"merge-and-shrink option", just like it recommends certain merge strategies to
match configurations mentioned in papers.

Álvaro, in case you are interested to see whether you are happy with the new
interface, I'd be happy if you had a look at:
https://bitbucket.org/SilvanS/fd-dev/pull-requests/14/issue655/diff

Feel free to start a code review and comment at bitbucket.

In the meanwhile, I'll run experiments as a safety check.

I think moving the parameters to MergeAndShrinkHeuristic is a good
solution for the moment. The only drawback I see is that it's
difficult to compose several shrinking strategies with different
thresholds (e.g., run bisimulation up to a limit of 500.000 abstract
states and then the bucket-based shrinking with a limit of 100.000
states). Anyway, I think is quite unlikely that anyone wants to try
such complex combinations in the near future so I think it is fine.

In the long run, I think it'd be good if all M&S operations shared the
same interface, taking as input a FactoredTransitionSystem and
returning another FactoredTransitionSystem (or just modifying it),
without taking additional parameters such as shrinking sizes or factor
indexes. This has the advantage that all operations are then easily
composable, simplifying the implementation of MergeAndShrinkHeuristic. 
Anyway, this could be implemented with a new layer of indirection and 
it is perfectly compatible with moving the parameters outside the shrinking 
strategy definition.

I found the issue: it's issue505. I'll a link from there to here, too.

I talked to Jendrik, and he said that "he thinks that he knows what you mean",
but that there is no issue for that topic.

I'll try to come up with a good way of updating the documentation.

I have no strong preference as long as the documentation remains consistent.
(That is, if you want to merge your current code, the documentation there would
need to be updated.)

> Perhaps some aliases would help with breaking the public interface in the
> future

Does this mean we should live breaking it for now? I can still ask Jendrik about
this issue of course.

(In case someone is interested, I already implemented the suggested changes, but
without a solution for the problem of updating the documentation of shrink
strategies, which use parameters such as max_states in their example command
line strings.

https://bitbucket.org/SilvanS/fd-dev/pull-requests/14/issue655/diff)

I agree this is a good idea conceptually, and I also agree that it's a pity that
this would change the public interface.

Perhaps some aliases would help with breaking the public interface in the
future, but then I think we need aliases that are more powerful than the ones we
currently have and can take parameters (for things like max_states).

There is an open issue about that somewhere. I think Jendrik might know about
it; I think he was one of the people behind it. Perhaps you can ask him about it
and link this issue to that one? Ideally we should have a link in both directions.

A drawback of moving the "max_states", "max_states_before_merge", and "treshold"
options from ShrinkStrategy to MergeAndShrinkHeuristic is that the heuristic has
even more options, and that the documentation of shrink strategies, which
currently mentions example configurations from the literature making use of
these options, would need to be moved out/split off. For the latter problem, I
don't see any obvious solution.

This is part of meta issue567.

Putting Álvaro on the nosy list due to our email exchange (feel free to remove
yourself, of course).

An item I have on my TODO list reads "compute shrink sizes not in shrink
strategy classes", and Álvaro recently communincated that he'd prefer that
shrink strategies have a public shrink-method that simply shrinks a single given
transition system, to a given (soft) limit. I think this is very reasonable. Our
current interface -- shrink(TS1, TS2) -- is tailored to the "shrinking before
merging" approach, and the order in which things happen in the merge-and-shrink
main loop should conceptually do not matter. Also, the computation of shrink
sizes, given the state limits, are somewhat unrelated to shrink strategies. A
shrink strategy prescribes *how* to shrink transition systems, not *when* and
*how much*. Hence I'd suggest to replace shrink(TS1, TS2) by shrink(TS, size)
and to move all "max_states"-related options to the merge-and-shrink heuristic,
which then also needs to take care of computing shrink sizes. This could also
benefit some merge strategies that would like to know about chosen size limits
of transition systems (such as MIASM).

Malte, do you think that this suggestion is a good idea or do you have different
ideas for this topic (or do you think everything is fine as it is right now)?