Ok, closing this one then.

It would be useful to do a general code review, but I don't think it should be
part of this issue, and I don't know how soon we could find time for it.

Ok, so this issue seems to be resolved for me. Should we plan a code review for
the M&S code in general and if yes, should that be part of this issue?

> I found no opportunities to profit (in terms of code reduction) from removing
> the option and still grouping states by h value. Is that the expected case?

Yes, that's fine. I think it's already a nice advantage to have one less option
to worry about, even if the code complexity only reduced marginally (by getting
rid of the option-parsing code, variable that stores the option, etc.).

I found no opportunities to profit (in terms of code reduction) from removing
the option and still grouping states by h value. Is that the expected case? If I
understood it correctly, you expected that removing the option and keeping only
the behavior that does not enforce grouping by h values could would have reduced
code complexity, but it seems that the other way round, this is not the case.

Yes, red means better with group_by_h. Good idea, as turning on the option also
does not hurt greedy bisimulation (where it was "originally" turned off).

> The impact of "group by h" is not negligible, I would say:
>
http://ai.cs.unibas.ch/_tmp_files/sieverss/2015-05-21-issue531-v2-group-by-h-comp.html
> In particular, it is not useful for the greedy bisimulation, but it is with
> regular. This is also reflected in the two M&S-portfolio configurations
accordingly.

If I read the "Summary" table correctly, red means "better with group_by_h" and
green means "better without group_by_h"? Then there is so much red and so little
green that we might consier always turning "group_by_h" on and getting rid of
the other possibility. What do you think?

The pure refactoring did not change the performance at all: 
http://ai.cs.unibas.ch/_tmp_files/sieverss/2015-05-18-issue531-base-v1-comp.html
http://ai.cs.unibas.ch/_tmp_files/sieverss/2015-05-21-issue531-v2-v1-comp.html

The impact of "group by h" is not negligible, I would say:
http://ai.cs.unibas.ch/_tmp_files/sieverss/2015-05-21-issue531-v2-group-by-h-comp.html
In particular, it is not useful for the greedy bisimulation, but it is with
regular. This is also reflected in the two M&S-portfolio configurations accordingly.

Unless we still want to remove the option, we need to at least briefly document it.

As (partly) discussed offline, I removed or replaced by comments most of the
TODOs. This led to some further refactoring in the shrinking classes, and to
amended output during the merge-and-shrink computation in general.

Next I will test the influence of the bisimulation option "group by h".

(We can also discuss the ones offline where discussion makes sense.)

I suggest you look over them. For the ones that are out of date or don't seem to
make sense, get rid of them. For the ones that you think we should deal with
now, let's open issues. For the remaining ones, leave them in for the future.

This is part of meta issue432.

In a "final" action item (memory management improvements excluded), we should go
through the code to find and fix TODOs/HACKs or address other (hopefully minor)
comments. Doing a code-review of the latest merge-and-shrink code in general
would also be a very good idea -- so many implementation details have changed in
the course of issue432.

There are currently 17 TODOs in the comments, and for most of them, I cannot
tell if they should be addressed or not (many of them are questions or comments
of the form "we could do this" or "maybe this could help"). How can we proceed
with this task?