Thanks. The buildbot showed a slight increase in memory usage but we decided to
accept that, so I'm marking this as resolved.

Thanks! I left two comments. Looks good to merge.

I added two entries in the change log entry, one for the support of CPLEX 12.9
and one for the changed OSI recommendation. I also updated the wiki.

The pull request is a simple 8 line change (resorting the library names) + 15
lines change log + 544 lines experiment files:
https://bitbucket.org/FlorianPommerening/downward-issues/pull-requests/61

Results look good. (I only looked at the summaries in the HTML report, but I think that's enough.) I'm not worried by the very slight negative trend and agree with recommending CPLEX 12.9.

Is there a pull request for this? I assume there should be one if we must add cplex1290 to the CMake file.

Regarding dynamic vs. static, I agree we should not force things, but if the order of the libraries expresses a preference, we cannot abstain from giving *some* preference anyway. In that case based on what you describe "cplex1290 cplex1280 cplex" is the order I would suggest.

That leaves the change log entry (msg8931).

> As a side note: I now have 10 concurrent installations of OSI on my grid
> account so I might be eligible for a free waffle iron.

:-)

Thanks, Florian! I cannot have a look at this immediately, but I will treat it
with priority. Before I forget it: we should also make a change log entry, which
should mention what changes in terms of supported and recommended CPLEX
versions, what this means for performance, and what users have to do.

The results are in. I attached the reports here instead of uploading them to
aifiles because there are so many combinations of configurations.

Generally, the data looks very noisy and I didn't see a clear benefit of one
version over the others. Dynamic linking tend to be faster and CPLEX 12.9 was a
little bit slower in general. Still, I think the differences are small enough to
change our documentation to the newer CPLEX version and merge this issue.

On the static/dynamic issue, I suggest to change the OSI installation
instructions to dynamic. However, our build script links with both OSI and CPLEX
and uses the first matching library it finds. With the updated build
instructions, there will only be a dynamic library of OSI available but CPLEX is
still available as both a static and a dynamic library. The choice which of
those is used is up to the build environment of the user. As I understand it, it
would be against CMake philosophy to control this. We could change the order of
the library names in our CPLEX find-script ordering "cplex" after "cplex1280",
"cplex1290" etc. This would express a preference of the dynamic library
(libcplex1290.so) over the static one (libcplex.a).

I made a mistake when looking at the installation. The installer does actually
come with the same kind of static library it used to in 12.8. We only have to
delete it in the Docker container to prevent the build to use it (CMake uses the
first matching library it finds). I ended up looking at the wrong layer of the
Docker image where the static library was already removed. So we actually have
four options: {static, dynamic} x {12.8, 12.9}.

I don't know a good way to test this with lab so I misused the revision cache to
start an experiment with those four options by recompiling in the revision cache
manually with the correct environment. The experiment runs diverse potential
heuristics, operator counting with SEQ and LM-cut constraints, and the optimal
partitioning of admissible LM-count on the optimal tasks.

As a side note: I now have 10 concurrent installations of OSI on my grid account
so I might be eligible for a free waffle iron.

I see. I would still recommend an experiment for this issue. For me, I wouldn't
want to say that we support CPLEX 12.9 if we never tested it. (The pull request
of course looks fine.)

I didn't intend to drop support for CPLEX 12.8. The change in the code that I
wan to merge just adds 12.9 as an option leaving the others intact.

The change to the wiki would be a change to the recommended version. I don't
have a strong opinion on this. Mainly I'm interested in the code change because
it blocks issue923.

It would be nice if someone (other than me) could test these instructions.

Also, it looks like this issue is "Drop support for CPLEX 12.8 and add CPLEX
12.9 support" given that 12.9 is hardcoded in the instructions. Is this the
intention? I think supporting only one version would be fine, but we should be
clear about this, also in the change log.

Can we also run an experiment that compares performance before/after this issue
similar to what we did with previous CPLEX upgrades and configuration changes? I
would not want to recommend a CPLEX version we never tested experimentally.

If I understand you correctly, there are three versions that could conceivably
be tested:

- CPLEX 12.8, linked statically (what we currently have)
- CPLEX 12.8, linked dynamically
- CPLEX 12.9, linked dynamically (what we would have after merging this issue)

I think it might make sense to have all three if the middle one isn't difficult
to run so that we can separately consider the effect of dynamic linking and of
the CPLEX version upgrade.

It's a simple change in the CMake find script and it requires some changes in
the wiki (http://www.fast-downward.org/LPBuildInstructions) to build the shared
library of OSI instead of the static one.

The changes to the repository are here:
https://bitbucket.org/FlorianPommerening/downward-issues/commits/d5f9dca8

In the wiki, I would change the configure call of OSI from
    ./configure CC="gcc"  CFLAGS="-pthread -Wno-long-long" \
            CXX="g++" CXXFLAGS="-pthread -Wno-long-long" \
            LDFLAGS="-L$DOWNWARD_CPLEX_ROOT/lib/x86-64_linux/static_pic" \
            --without-lapack --enable-static=yes \
            --prefix="$DOWNWARD_COIN_ROOT" \
            --disable-zlib --disable-bzlib \
            --with-cplex-incdir=$DOWNWARD_CPLEX_ROOT/include/ilcplex
            --with-cplex-lib="-lcplex -lm -ldl"

to 
    ./configure CC="gcc"  CFLAGS="-pthread -Wno-long-long" \
            CXX="g++" CXXFLAGS="-pthread -Wno-long-long" \
            LDFLAGS="-L$DOWNWARD_CPLEX_ROOT/bin/x86-64_linux" \
            --without-lapack --enable-static=no \
            --prefix="$DOWNWARD_COIN_ROOT" \
            --disable-zlib --disable-bzlib \
            --with-cplex-incdir=$DOWNWARD_CPLEX_ROOT/include/ilcplex
            --with-cplex-lib="-lcplex1290 -lm -ldl"

Malte, can I merge these changes? It would help with testing the containers for
issue923.

Now that the academic version of CPLEX 12.9 is available we should update our
build scripts to find and accept it. From a first look at the installation, the
installer doesn't come with a static library any more changing a few paths where
we look for the library.