OK, no automatic change, so looks fine. Let's not look into it further until it
pops up again.

Let's test it.

That might be due to the fact that my status still says "chatting" after
reloading the page, although you (again) changed it to "resolved". Without
manually changing it to "resolved", as I now did, I probably would again set it
to "chatting" by sending this comment.

Hmmm, looks like we somehow reactivated the old tracker (mis-) feature where
commenting on a resolved issue automatically changed its status to "chatting".
Will look into it.

Thank you for all the work Florian!

I think the email should also to the Fast Downward list, because it also changes
the way "non-dev" users use Fast Downward.

Great. Thanks again to everyone.

I updated the buildbot config, everything runs smoothly :)

One thing I had to do manually was setting the environment variable DOWNWARD_CPLEX_ROOT as 
detailed in http://www.fast-downward.org/LPBuildInstructions. Perhaps this is useful for other LP 
users as well.

I merged, updated the documentation on the wiki and wrote an announcement. Could
someone with access to the buildbot exchange the current config with the one in
the repository?

Should I also send the email to the main Fast Downward list as it also concerns
users of Fast Downward?

Sure! Once that's done, please let everyone on downward-dev know what changes
for them.

Jendrik reviewed the CMake files as well (thanks!). May I merge this?

If both of you have no preference I will leave it as it is for now. Thanks for
the review Jendrik.

That just leaves the 9 cmake files. I can explain what they do if that helps
with a review (its also documented in the files):

src/CMakeLists.txt
  * main file, sets up cmake and creates projects for VAL, preprocess and search
src/{VAL,preprocess,search}/CMakeLists.txt
  * set compiler flags, find libraries, define project and source files
src/search/FastDownwardPlugins.cmake
  * For the main planner, we have the list of source files in an extra cmake
file to keep
    things organized (this should be the only cmake file most people will ever
have to
    modify). This file also groups source files into plugins.
src/cmake_modules/FastDownwardMacros.cmake
  * Helper functions that are used in the other files.
src/cmake_modules/Find{Clp,Cplex,OSI}.cmake
  * These scripts should find the respective libraries on all systems. They
mainly contain
    large case distinctions to account for the different default locations on
different
    setups.

I think, for someone new to cmake, it would be easiest to first look at
src/CMakeLists.txt and src/VAL/CMakeLists.txt and see how VAL is built. After
that, look at src/preprocess/CMakeLists.txt and some of the functions in
src/cmake_modules/FastDownwardMacros.cmake for the preprocessor; then the rest
of the files for the search. I'm happy to explain everything and add
documentation to everything that is not clear.

My comment about --build options for build.py was just an idea, I also have no 
preference for one or the other :)

I have read the documentation you linked to and made some very small changes. The 
docs look good to me.

No preference.

I prefer typing "./build.py release32" to "./build.py --build=release32", but I
can change it if thats just me.

To further unify the option syntax of the two scripts, should we make the 
build.py arguments release32, debug32, etc. explicitly optional like in the 
fast-downward.py script, i.e., change "./build.py debug64" to "./build.py --
build debug64"? Contrary to fast-downward.py the build.py script would allow 
that the --build option is passed multiple times. Then both scripts would have 
--debug and --build options.

Ok, I added the option "--debug" to build.py which behaves like specifying the
build type "debug32". I left the option "--all" of build.py as it was, so I did
not use it in the buildbot config. I agree it makes sense to keep it for local
tests.

Jendrik, could you also have a look at the new documentation?
  http://www.fast-downward.org/ObtainingAndRunningFastDownwardcmake
  http://www.fast-downward.org/PlannerUsagecmake

> We were talking about fast-downward.py.

For both options you mentioned? Then I still don't know what "./fast-downward.py
--all" does. :-)

> Should we add an option to build.py that is called "debug" and acts like
> "debug32"? If so, should we also add an option "release" that acts as
> "release32" or is it enough that not giving any option acts like this?

If we have a debug option there, I'd call it "--debug" to make clear that it is
a script option, not a build name (i.e., to avoid mixing the namespaces).

Whether we should have it: I think it would be useful to have a way to make a
debug build that doesn't require mentioning bitness. If the others would rather
not have this, I won't insist.

Regarding --release: if it's the default, I don't see us using it or
recommending its use, so I'd argue for YAGNI. But if we have --debug in build.py
and in fast-downward.py, then we should have --release in neither or both.
(Currently, fast-downward.py doesn't have --release, right?)

> About the "all" option: the build bot config in the branch is currently not set
> up to use all, so we can see which step an error occurred in. Should we change
this?

No strong preference. I usually find I'll have to look into the log file anyway
to find out what went wrong with the build. (If the build type is not easily
discernible from the log file, it should be.)

But even if we don't use --all in the buildbot, I would use it locally to make
sure I didn't break the build when fiddling with things that might behave
differently in different versions of the code, so I'd appreciate the convenience
of having it around. But if nobody else thinks it's useful, I don't think we
need to have it.

We were talking about fast-downward.py. Currently, the way to build and run a
debug build in the issue branch is:

  ./build.py debug32
  ./fast-downward.py --build=debug32 ...

And we have the shortcut option for fast-downward.py that does the same thing:

  ./fast-downward.py --debug

Should we add an option to build.py that is called "debug" and acts like
"debug32"? If so, should we also add an option "release" that acts as
"release32" or is it enough that not giving any option acts like this?


About the "all" option: the build bot config in the branch is currently not set
up to use all, so we can see which step an error occurred in. Should we change this?

Ah, I didn't realize you were talking about build.py. I thought you were talking
about fast-downward.py.

To currently build a debug build, we write:

    ./build_all debug

As far as I understand, in Florian's code, the equivalent call is

    ./build.py --debug

and the suggestion is to remove this alias and require

    ./build.py --build=debug32

?

I'm not sure this is an improvement. "Give me a debug build" is a common
requirement, and I'm not sure if it's a good idea to require people to know that
Fast Downward prefers 32-bit builds to perform this step in the correct way, as
this is a bit of an implementation detail most users should not care about.

For example, I can see people accidentally using --build=debug64 because they
think that's the right thing to do on a 64-bit machine. If we have a "default
release build" (release32, which you get by passing no options to build.py),
then I think it also makes sense to have a "default debug build".

> --all: "./build.py --all" builds all predefined build configurations. At the
> moment, I don't really see a use case for it.

The main use case is to test if all predefined build types work. For example, we
can use this in the buildbot instead of having to manually specify all builds we
want to test. This has some advantages; e.g., we can change the set of supported
builds without changing the buildbot configuration (for example if at some point
in the future we can make the 64-bit build as good as the 32-bit one and then
always use native builds).

--debug: I think explicitly passing --build=debug32 or --build=debug64 is clearer 
than --debug and corresponds to what one needs to pass to ./build.py. 

--all: "./build.py --all" builds all predefined build configurations. At the 
moment, I don't really see a use case for it.

I frequently use "--debug" because I usually want to enable assertions when
trying out things or debugging. If the consensus is to remove it, I guess I
could live without it, though.

I'm not aware of an option called "--all". What does it do?

Thanks Jendrik. I fixed everything you mentioned except removing the "--all" and
"--debug" options. Malte, we currently do not use these options on the buildbot,
should we still support them?

I have reviewed all non-cmake files.

I moved the driver script in a new branch (based on default) which makes the
diff more useful:
https://bitbucket.org/flogo/downward-issues-issue67/pull-requests/2

The main problem is that none of us (me included) is an expert on cmake. What do
you want the review to look like, how detailed should it be?

Maybe someone can start reviewing the non-cmake files? There is a bunch of
small, easy changes there; mostly caused by the new way of calling Fast
Downward. Any volunteers?

Thanks a lot, Florian! Please find a reviewer and then merge. However, it looks
like many changes can't be reviewed with this bitbucket link due to changed
paths that break the side-by-side comparison (e.g. for the driver scripts).

After some further experimentation we found out that there were two main
differences responsible for the reduced performance with cmake: dynamic linking
and the order in which object files are given to the linker. I changed the cmake
file to force static linking to fix the former. For the order of files, I found
no recommendations what makes a "good" order. With some older linker versions,
files that define a function must be listed after the files that use the
function. I tried to approximate this order by reversing the list of source
files after computing it. This has the effect of listing all active plugins
first, followed by the core planner files. A better order could potentially
bring more benefit, but this order seems to have brought performance back to
acceptable levels (see reports and plots below). If someone wants to investigate
this in more detail, one place to look are files that occur in the same order in
the old makefile and the reversed cmake list, but in a different order in the
non-reversed cmake list. The old makefile order and the non-reversed cmake order
are not too different, so there should not be too many such files.

The experiments look good (total time actually slightly decreases on average)

http://ai.cs.unibas.ch/_tmp_files/pommeren/issue67-v4-issue67-v4-base-issue67-v4-compare.html
http://ai.cs.unibas.ch/_tmp_files/pommeren/issue67-v4-total-time-blind.png
http://ai.cs.unibas.ch/_tmp_files/pommeren/issue67-v4-total-time-lmcut.png
http://ai.cs.unibas.ch/_tmp_files/pommeren/issue67-v4-total-time-lm_zg.png

I think this issue is now ready for review. The code is at
  https://bitbucket.org/flogo/downward-issues-issue67/pull-requests/1/issue67/diff
and the new documentation at
  http://www.fast-downward.org/ObtainingAndRunningFastDownwardcmake
  http://www.fast-downward.org/PlannerUsagecmake

Jendrik already had a look at the build.py script and the changes in the driver.

I experimented locally with compiler and linker options. The compiler options
used by the last make and cmake version (the ones with -rdynamic) produced an
identical binary when the object files were identical. Since there is still a
time difference between the two versions, I looked at the compile step again.
Here, I found that even the same call to the compiler (using only absolute
paths) called from two different directories, ended up producing different binaries.

Unfortunately, the resutls are not completely clear. We did get a similar
difference between versions without LP solver, which makes the analysis a bit
simpler, though.

Between the static-make version (E) and the rdynamic-cmake (D), we indeed got a
difference of average total time score of -0.13. The same versions with LP (A
and B) had -0.12.

Changing static to dynamic in the make version (E -> F) reduced the average
total time score by 0.08. Adding the flag -rdynamic (F -> G) in the makefile had
no effect on the time score (but did increase memory usage slightly).

This means there is still a difference of 0.05 between the makefile version
using -rdynamic and the cmake version, and I don't know why. All remaining
differences in the g++ call seem trivial to me.

I don't know enough about cmake to do this quickly. I suggest we change version
B instead and make it more like A. I looked at the differences in the linker
command again and found three main differences:

1) the OSI libraries were linked statically before, and dynamically with cmake
2) the makefile added --static-lib-gcc, the cmake build doesn't
3) the cmake build adds -rdynamic

I started another experiment with the following versions:

D) "v3": like A/v2 but without the LP solver
E) "v3-base-same-source": like B but without the LP solver
F) "v3-base-same-source-dyn": like E but without --static-lib-gcc
G) "v3-base-same-source-rdyn": like F but with -rdynamic

My guess is that D and E will still have the difference because blind search
should not depend on the LP solver. I also hope that D and G will not have the
difference, because they have only trivial differences in their compiler calls
left. If that is the case, the difference would be either between E and F or
between F and G. We'll see when the experiment is done.

> Looks like the change really is caused by the compiling/linking and not by the
> code. Since the compile parameters looks so similar, I think the problem is the
> linking but I don't understand it enough to pinpoint the issue.

I would suggest first verifying the hypothesis that it's the linking command
that is at fault. Suggestion: we currently have versions A and B. Add a new
version A' which is like A but using the linking command of B, and a new version
B' which is like B but using the linking command of A. Compare the four versions.

As discussed offline, I re-ran the experiment with a version of the base code,
that merged all the minor changes that shouldn't make a difference. There are
three revisions in these experiments:

  A) "v2": the new code compiled using cmake
  B) "v2-base-same-source": the new code compiled using
     the makefile. Code is identical to A.
  C) "v2-base": the last revision on the default branch
     merged into the issue branch compiled using the makefile
     (common ancestor of A and B)

Comparing A to B:
http://ai.cs.unibas.ch/_tmp_files/pommeren/issue67-issue67-test-issue67-v2-base-same-source-issue67-v2-compare.html

Comparing B to C:
http://ai.cs.unibas.ch/_tmp_files/pommeren/issue67-issue67-test-issue67-v2-base-issue67-v2-base-same-source-compare.html

Comparing A to C:
http://ai.cs.unibas.ch/_tmp_files/pommeren/issue67-issue67-test-issue67-v2-base-issue67-v2-compare.html

Looks like the change really is caused by the compiling/linking and not by the
code. Since the compile parameters looks so similar, I think the problem is the
linking but I don't understand it enough to pinpoint the issue.

I just checked the revisions that ran in the experiment again, and it seems I
used a wrong tag. Between the revision on the default branch, and the one on the
cmake branch, there was one more commit merging issue573. But this should not
influence anything. Here is the corrected link for the diff:

https://bitbucket.org/flogo/downward-issues-issue67/branches/compare/issue67-v1%0Dissue67-v1-base#diff

Thanks for your response Malte, I just want to say that while I appreciate the
review, this can of course wait until after your holidays.

> I assume all configurations have been run at the same time, so that we can rule
> out the background load of the grid as a contributing factor?

Yes, and the run script uses the randomized order.

> In that case, can you please send the compiler options used in the experiment,
> before and after? (I can look up the old ones, so just the new ones would be
> enough.) Are the same compiler versions used in both cases?

I attached the logs for the new and old code. In the file
compile-cmake-shortened.log and compile-make.log I replaced absolute paths to
the search folder by relative paths, removed cmake log messages and introduced
some newlines so the files can be compared more easily (I didn't change the
order or anything else). compile-cmake-original.log contains the unchanged
output from cmake (in both cases I remove the object file for planner and
recompiled).

The linker calls look different, they actually link all the LP libraries
statically in the old version.

> PS: If you can point to a diff or pull request that shows the differences
> between the two tested versions, that would also be very useful.

https://bitbucket.org/flogo/downward-issues-issue67/pull-requests/1

> I tried remove the following code from 
> src/search/CMakeLists.txt in latest codebase

Thanks. That makes two successful tests without this special case. I'll remove it.

Hi Florian,

Sorry for late response. I tried remove the following code from 
src/search/CMakeLists.txt in latest codebase:

  if(APPLE)
      include_directories("/opt/local/include/")
  endif()

Everything is good: compile success, test success
Build log attached.

PS: If you can point to a diff or pull request that shows the differences
between the two tested versions, that would also be very useful.

I assume all configurations have been run at the same time, so that we can rule
out the background load of the grid as a contributing factor?

In that case, can you please send the compiler options used in the experiment,
before and after? (I can look up the old ones, so just the new ones would be
enough.) Are the same compiler versions used in both cases?

We now have successful tests on Windows (Visual Studio and commandline), OS X
(XCode and commandline), Ubuntu (clion, qtcreator, commandline), Arch linux
(commandline), and on our grid. Thanks for testing, everyone.

Jendrik also updated Lab to deal with the new system (currently in the lab
branch "cmake"), which also works on our grid. Thanks, Jendrik.

I ran an experiment to compare the last current default branch with the issue
branch and got some unexpected changes:

http://ai.cs.unibas.ch/_tmp_files/pommeren/issue67-issue67-issue67-v1-base-issue67-v1-compare.html

Memory usage increases by a (more or less) constant 1592 KB. My guess is that
the reason for this are the added *.cc files, but I'm not sure because 1.6 MB
seems like too much for that change.

Total time also increases in a majority of cases, usually by less than 10%. I
don't know where this is coming from.

In total we lose 2 tasks in coverage and gain 1, but all three tasks took very
close to 30 minutes in the other config, so this could also be noise.

The code changes in this issue are minor, and the build system uses mostly the
same flags as before to call the compiler. Some flags are in a different order,
some use absolute paths instead of relative paths, one additional include
directory for CPLEX is used (but probably not needed).

Can anyone review this?

Great, thanks! It seems cmake did not find the flex include dir and thus did not
build VAL, but this is something, we are aware of:
http://www.fast-downward.org/ObtainingAndRunningFastDownwardcmake#Compiling_on_Mac_OS_X.

If you want to try more, you could try to generate an XCode project and try to
build from XCode:
http://www.fast-downward.org/ObtainingAndRunningFastDownwardcmake#Manual_Builds

Another thing I would be interested in, is whether we need the special case for
tree.hh that is something we carried over from the old Makefile. To test this,
you can remove the following lines from src/search/CMakeLists.txt and try to do
a build (remove any old builds before you try, because cmake caches some settings):

  if(APPLE)
      include_directories("/opt/local/include/")
  endif()

Hi Florian,

I don't have cmake installed. 
Build success after I installed cmake.
I also attached the build log file incase you want to checkout something.

I also run script: "./fast-downward.py benchmarks/airport/p01-airport1-p1.pddl --search 
"astar(lmcut())"".
And it succeed.

Let me know if you need me try out something else.

Hi Xiao,

did you install cmake? If so, can you make sure that it is on your PATH?

Hi Florian,

It failed at very beginning, here is my console log:

xiao:~/code/forks$ cd downward-issues-issue67/
xiao:~/code/forks/downward-issues-issue67$ hg update issue67
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
xiao:~/code/forks/downward-issues-issue67$ ./build.py -j4
Building configuration release32
Traceback (most recent call last):
  File "./build.py", line 80, in <module>
    main()
  File "./build.py", line 76, in main
    build(config_name, CONFIGS[config_name], make_parameters)
  File "./build.py", line 59, in build
    cwd=build_path)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 535, in check_call
    retcode = call(*popenargs, **kwargs)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 522, in call
    return Popen(*popenargs, **kwargs).wait()
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 709, in __init__
    errread, errwrite)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1326, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory
xiao:~/code/forks/downward-issues-issue67$ gcc -v
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 6.1.0 (clang-602.0.53) (based on LLVM 3.6.0svn)
Target: x86_64-apple-darwin14.4.0
Thread model: posix

I extended the option "--build" of the driver a bit to also accept paths that
directly lead to the binaries. This means the the following are all equivalent:

  ./fast-downward.py --debug
  ./fast-downward.py --build=debug32
  ./fast-downward.py --build=relative/path/to/planner/builds/debug32/bin
  ./fast-downward.py --build=/absolute/path/to/planner/builds/debug32/bin

This change will be helpful to call the driver script from an IDE, which
generate the binaries in more complex paths (e.g.,
builds/xcodeproject/bin/Debug). The reason for these paths is that IDE projects
are multi-configuration generators in cmake and include Debug and Release in one
directory.

It might also help with lab, because it means that the binaries don't have to be
in a 'builds/<name>/bin' directory.

Silvan and I tested the buildbot today and now it would work with the code in
this branch. We switched it back to the default branch until we merge this issue.

Manuel also did a first test on Mac OS X and we managed to get everything except
VAL to compile. VAL did not compile because flex and bison where not found, so I
changed the cmake files to skip compiling VAL if that is the case. I don't think
there is much we can do on our side if cmake does not find flex/bison, the paths
just have to be set up correctly before calling cmake.

I think the last remaining tasks here are to update lab, do a code review and
(optionally) more testing with other system configurations if anyone is missing
their favourite configuration.

Out-of-tracker response from Erez:
> Wow, I don't remember this at all.
> Please, feel free to get rid of it.
> 
> Cheers,
> Erez.

Ok, I removed doc.h and landmarks/landmark_types.h, and added *.cc files for the
other lonely header files.

The cmake setup now (unconditionally) adds a x.h file for every x.cc file in the
sources. This acually means that we could leave some headers without a *.cc file
(e.g., utilities_windows.h) and just list them in the list of sources next to
all *.cc files. What we don't support this way are *.cc files without *.h files.

I think this way is actually more intutive than listing header files and adding
*.cc files, because the *.cc files are the files that are acutally compiled, the
headers are just copied into them before compilation.

If I recall correctly, doc.h was created for a Doxygen effort by Erez that
hasn't seen any love since 2009 or so. I suggest to delete it, since I don't
think it's all that useful currently and we haven't been maintaining it.

I wouldn't add a header for the main file but keep that as a special case.
That's the way our current Makefile works, too. (The source files are planner.cc
plus cc files for each of the listed header files.)

Currently, the following header files have no *.cc file:

1) plugin.h
2) priority_queue.h
3) landmarks/landmark_types.h
4) doc.h
5) utilities_windows.h

For 1) and 2) a *.cc file makes sense (they define templated types).

File 3) contains a single typedef and the header is included in a lot of places.
The type is only used in landmarks/landmark_factory_zhu_givan.{h,cc} so it could
just be moved there.

File 4) contains just one comment with what looks like structured text (/*!
\mainpage Fast Downward Index Page ... */). Is this used anywhere? Maybe we can
rename it to README, move it somewhere else, or delete it?

Finally, 5) wrapps the include for windows.h and I think this might be a
legitimate reason for a header without *.cc file, but we can also add a *.cc
file here.

We could then loop over *.cc files in cmake and add each header to the list
(should we also add a header for planner.cc?)

> > Why don't we just require that each header file also has a (possibly empty) .cc
> > file?
> 
> Yes, that was the gist of what I meant.

Should we do this in this issue and then only list *.cc files in the list?


Unrelated to this, I have copied and updated some documentation in the wiki.
Once we merge this issue, I would copy them over the original files. Can someone
review this?

http://www.fast-downward.org/ObtainingAndRunningFastDownwardcmake
http://www.fast-downward.org/PlannerUsagecmake

I have now assigned issue481 to myself; it's been open since September, so I
guess nobody else was terribly keen on working on it. :-) It's hard to predict
how long it will take though, so please don't wait for that issue.

> Why don't we just require that each header file also has a (possibly empty) .cc
> file?

Yes, that was the gist of what I meant.

> I think we agreed to remove the open list hack anyway and wanted to move the
> templated code into the header files. Of course this doesn't have to be done in
> this issue.

There are two unrelated points here:

A) The *.h files of the open lists include their *.cc files.
B) The open lists are not pluggable. (That is, it's not possible to selective
add or remove an open list, and all open lists must be explicitly mentioned in
an unrelated part of the code.)

A) is easy to change, but the real problem here is B), which is issue481. I
don't think B) would be terribly difficult to change, but it would cause a bit
more work, and so far nobody has volunteered to do it. :-)

I see this as unrelated to the cmake migration and would rather proceed without
waiting for issue481. In any case, if we want to make open lists pluggable, we
don't want a single plug-in for all open lists; that would be rather pointless,
as one alwas needs at least one open list. Rather, the individual open lists
should be individual plugins.

Why don't we just require that each header file also has a (possibly empty) .cc 
file?

I think we agreed to remove the open list hack anyway and wanted to move the 
templated code into the header files. Of course this doesn't have to be done in 
this issue.

I put the open lists and maybe_core plugins back into the core. Open lists
behave a bit weird because their header files include their *.cc file. The build
(like the Makefile before) now compiles the *.cc file, which does nothing, and
includes the header in the relevant places, which includes the actual code.

About listing all files: we could avoid this by looping through the sources and
adding header files if they exist. This has the same disadvantages as globing,
though. It would also add some complexity to the cmake file.

> Yes, CMake will use included header independently of whether they are listed in
> the sources. Technically, cmake does not know about the header in this case, but
> it builds each file using the directory of the file as a working directory. The
> header is not part of the call to the compiler but since it is included with a
> relative path, the compiler will find it. I think this was the same with our
> Makefile.

Ah, I didn't know that, but I guess it doesn't really change the main point I
wanted to make: the open list code is not optional. The open list source files
are needed to compile the planner, and they will be included whether or not the
OPEN_LIST plugin is selected. Hence, from what you describe, it seems that the
OPEN_LIST plugin has no effect. I think then it would be better to include these
source files in the core and get rid of the plugin. Otherwise, from what you
describe I would expect a user to think that the source files listed for the
modules they choose are enough to build the planner, which isn't true.

> Otherwise, header files where no *.cc file exists (plugin.h, priority_queue.h,
...)
> are not part of the project.

I'm not sure if such files (headers without cc files) are supposed to exist in
the first place. :-) I think it's a bit annoying to list every file name twice,
but I guess it's not a big deal.

> I'm surprised to see that the open lists are not part of the core, given that
> plugin.h includes all the open lists. Is it really possible to build the planner
> without the OPEN_LISTS plugin?

Yes, CMake will use included header independently of whether they are listed in
the sources. Technically, cmake does not know about the header in this case, but
it builds each file using the directory of the file as a working directory. The
header is not part of the call to the compiler but since it is included with a
relative path, the compiler will find it. I think this was the same with our
Makefile.

> Is it necessary to list both the *.cc and the *.h file always? Don't all build
systems detect the dependent headers anyway?

No, not really (for the same reason). However, when we use cmake to generate
project files for IDEs, those will include only the files that are specifically
mentioned. I thought it would be better to be explicit here and list all files
that belong to the project. Otherwise, header files where no *.cc file exists
(plugin.h, priority_queue.h, ...) are not part of the project. Its possible to
collect all headers with glob, but this has other disadvantages
(http://stackoverflow.com/a/18538444/892961).

It's not necessary to get the plugin structure 100% perfect from the start. I
would everything in MAYBE_CORE to the core. Otherwise we have to list lots of
stupid dependencies everywhere. For example, while it's true that we can compile
the planner without the various classes related to the state representation, it
won't be possible to instantiate a search algorithm without them.

I'm surprised to see that the open lists are not part of the core, given that
plugin.h includes all the open lists. Is it really possible to build the planner
without the OPEN_LISTS plugin?

Is it necessary to list both the *.cc and the *.h file always? Don't all build
systems detect the dependent headers anyway?

I merged the patch. Thanks, Jendrik!

I also added in a plugin mechanism and started seperating the files into plugins
that can be compiled by themselves (or at least depend only on a small number of
other plugins). If a plugin is enabled, all its dependencies will be enabled as
well.

Malte, can you have a look at the list of plugins? In particular there are some
files that sound like they should be in the core, but the core compiles without
them (the plugin called "MAYBE_CORE"). You can find the list at
https://bitbucket.org/flogo/downward-issues-issue67/pull-requests/1/issue67/diff#chg-src/search/FastDownwardPlugins.cmake

Merging the master default branch into the issue67 branch should also fix the 
issue in driver/util.py.

Compilation with NMake should now work on Windows. In case anyone wants to try:
Open your VS command prompt (usually in C:\Program Files (x86)\Microsoft Visual
Studio <VERSION>\Common7\Tools\Shortcuts\VS<VERSION> x86 Native Tools Command
Prompt) and then call "python ./build.py" in the repository root. Running the
planner with "./fast-downward.py ..." should work as well if you comment out the
warning in driver/util.py. Once we permanently fix the warning, the buildbot
should also be able to compile and run the planner on Windows without the
additional repository.

The build failed earlier, because of a wrong forward declaration in VAL. The
current version of VAL does not have the file with the forward declaration
anymore, so it will not have this problem. We probably should update our version
of VAL at some point, but for this issue, the fix should sufficient.

> Jendrik, can you look into this and see
> what kind of changes would be required to support the new scripts?

Sure. I'm on holiday next week, but afterwards I'll have a look.

Thanks. I imported the patch in default and merged this back to the issue,
removing the ignored warning.

Also thanks for the tutorial on Flex/Bison with Visual Studio, I think that
saved me an hour of work.

Patch attached for the extra overrides required for clang-3.6.  (As an aside, I
left address sanitizer switched on, which picked up some memory leaks during the
domain transition graph code.  Non-urgent, at least compared to memory leaks
during search.) 

To build with VS2015 - I generated a solution, and built it.  I had to do some
CMake gymnastics, but in my experience that's normal when trying to get
flex/bison to work on Windows:

- Load the CMake GUI
- Point it to the source code and a build directory
- Press Configure, choose the VS 2015 generator
- Set CMAKE_PREFIX_PATH to point to bison and flex; Configure again
- Close the GUI; open CMakeCache.txt; manually correct FLEX_INCLUDE_DIR to point
to the include directory containing FlexLexer.h
- Reload the GUI; configure; generate.
- Load Visual Studio; load the solution; build it.

The build now ignores the three warnings you mentioned. Could you pull, update,
and try again to see if it compiles now?

Thanks for testing, Andrew. We currently ignore some errors on Windows and I
will add the ones from your log to the list. Our plan is to get a more detailed
look at each of these ignores and then try to get rid of them one by one. I can
open an issue for that.

Out of curiosity: how did you build with Visual Studio? Did you use nmake or
generate a solution? I only tested nmake so far (and not with this version of
the makefile), so I'm interested to hear if/how that worked out.

>> 2) Currently ./fast-downward.py --cleanup is a poor replacement for ./cleanup.py
>
> Are you suggesting to keep the ./cleanup.py script until we have tab completion,
> or did you just want to document our decision to move it into the driver?

I just wanted to document the things I noticed when working with the new branch.
I don't think there's an immediate need for any particular change.

> 1) fast-downward.py is a bit over-eager in printing the selected build.

I changed it to only show when the search or preprocessor is used (the
translator is not affected by the build).

> 2) Currently ./fast-downward.py --cleanup is a poor replacement for ./cleanup.py

Are you suggesting to keep the ./cleanup.py script until we have tab completion,
or did you just want to document our decision to move it into the driver?

> 3) In the cases where fast-downward.py fails to find a file [...] get rid of
the traceback

I'll open an issue.

> Now that running the build doesn't show the compiler options any more, is there
> an easy way to find out how the compiler is invoked?

Yes, by using 'VERBOSE=1' as a parameter for make. Since ./build.py forwards
every parameter that is not a build name to make, you can use:
  ./build.py VERBOSE=1


> Also, is there an easy way to change options such as the optimization level?

I don't know of an easier way than editing the cmake file
(cmake_modules/FastDownwardMacros.cmake). This seems like something that should
be possible with cmake, but that I set up in the wrong way. Though I don't know
how to do this correctly. We explicitly set the flags for the different build
types, but that also means that we overwrite any user configuration like
  cmake ../../src -DCMAKE_CXX_FLAGS_RELEASE="-O2"

In general, you can add new flags with 
  -DCMAKE_CXX_FLAGS="-O2"
but this will only add the flag "-O2", not remove the flag "-O3".

Thanks, Andrew! We should fix clang's warnings, as we want to use override
consistently, just haven't converted all the old code yet.

Regarding the Visual C++ warnings, I guess the easiest thing would be to fix
these, too.

Can someone open an issue for these?

Tried with the most recent (i.e. fussy) compilers I have to hand.  Build system
is fine, but by default won't compile due to new compiler warnings.  Log
attached, notes below.

-- Results for clang 3.6 --

Build failed due to 'inconsistent missing override' warnings - i.e. where some
functions in a class use the C++11 override annotation, but others do not, even
if they are overriders.

-- Results for Visual Studio 2015 --

It doesn't like local declarations that hide class members; or earlier local
declarations.

The 'unknown compiler version' message is from Boost - it's not a compilation
blocker.

Build works fine for me. :-)

Some notes:

1) fast-downward.py is a bit over-eager in printing the selected build. For
example, "./fast-downward.py --cleanup" prints the selected build and nothing
else. "./fast-downward.py" without options print the selected build and then a
usage error.

2) Currently ./fast-downward.py --cleanup is a poor replacement for ./cleanup.py
because we don't have tab completion for fast-downward.py's options. It would be
nice to add that at some point, though that's unrelated to the issue. (I'm only
bringing it up because the issue deletes cleanup.py.)

3) In the cases where fast-downward.py fails to find a file (e.g. if we failed
to compile the code first), it prints a nice error message regarding what has
gone wrong, but it also prints a traceback. Perhaps we should get rid of the
traceback at some point. (But this has nothing to do with this issue, really.)

Now that running the build doesn't show the compiler options any more, is there
an easy way to find out how the compiler is invoked? Also, is there an easy way
to change options such as the optimization level?

If cmake is not installed on your machine, build.py fails as follows:

$ ./build.py -j4
Building configuration release32
Traceback (most recent call last):
  File "./build.py", line 67, in <module>
    main()
  File "./build.py", line 63, in main
    build(config_name, CONFIGS[config_name], make_parameters)
  File "./build.py", line 46, in build
    cwd=build_path)
  File "/usr/lib/python2.7/subprocess.py", line 535, in check_call
    retcode = call(*popenargs, **kwargs)
  File "/usr/lib/python2.7/subprocess.py", line 522, in call
    return Popen(*popenargs, **kwargs).wait()
  File "/usr/lib/python2.7/subprocess.py", line 710, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1335, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory


This is not very descriptive, as it fails to see *which* thing it couldn't find.
Perhaps we can catch the error and give a better error message? I recall we
already did things like this in the past when flex or bison were not found.

As Erez just pointed out, you also need to update to the correct branch after
cloning the repository (I wish I could edit my previous post...):

  hg up issue67

Yes, sorry. Installing cmake should be sufficient for a basic build:
  sudo apt-get install cmake

If you want to use ccmake, ninja or clang, you have to install them as well:
  sudo apt-get install cmake-curses-gui
  sudo apt-get install ninja-build
  sudo apt-get install clang-3.5

Any information on extra software we need to install first? It would be good to
mention at least the Ubuntu dependencies on our build instructions wiki page. I
assume we need some cmake page; perhaps additionally something for ccmake?
Anything else?

Hi everyone,

I have a more-or-less(*) working version of the cmake build system and am
interested if it works for everyone. As a first step, I would like to ask you
(the subscribers of this issue) to clone the repository, try to build and run
the planner, and let me know whether it worked. Specifically, it would be good
to test different setups: can someone test under Windows, OS X, with CLP
installed, with clang, using ninja, ...?

The following steps should work on linux and OS X (see detailed instructions
further down for Windows):

  hg clone ssh://hg@bitbucket.org/flogo/downward-issues-issue67 issue67
  cd issue67
  ./build.py -j4
  ./fast-downward.py benchmarks/airport/p01-airport1-p1.pddl --search
"astar(lmcut())"

This will build the standard build release32 in builds/release32. You can build
other predefined builds by changing step 3 to one of

  ./build.py debug32 -j4
  ./build.py debug64 -j4
  ./build.py release64 -j4

If you want to try different settings either add a config in ./build.py or do
the following:

  mkdir -p builds/mybuild
  cd builds/mybuild
  cmake ../../src -DYOUR_CMAKE_OPTIONS_GO_HERE
  make

Possible options
  -DPLUGIN_BLIND_SEARCH_HEURISTIC=False
    Switch off the blind heuristic.
    See src/search/FastDownwardPlugins.cmake for other options.
  -DUSE_LP=False
    Force the planner to not use an LP solver even if one is installed.
  -DCMAKE_BUILD_TYPE=DEBUG
    Other options: RELEASE (default), PROFILE
  -DALLOW_64_BIT=YES
    If you want to do a 64bit compile, you might also have to set up your
    compiler in the right way, e.g., with -DCMAKE_CXX_FLAGS="-m64".

Or you could try a different generator
  -GNinja
    Use "ninja" instead of "make" in step 4.
  -G"NMake Makefiles"
    Windows command line compile. Open the developer shell for your compiler
    and then use nmake instead of make in step 4.
  -G"Visual Studio 12 2013"
    This should generate a solution for Visual Studio 2013.

Or try a different compiler
  CC=clang-3.5 CXX=clang-3.5 cmake ../../src

You can also try out the cmake commandline gui "ccmake" to edit the available
options in a build directory.


If you want to run your custom build ("mybuild" in the example above), use
  ./fast-downward.py --build=mybuild benchmarks/airport/p01-airport1-p1.pddl \
                     --search "astar(lmcut())"


Any comments are welcome. If you used cmake before, I would also be happy to
hear your opinion on the cmake files themselves.


(*) Caveat:
I am still working on updating all the scripts in misc/* to do the right thing,
now that the way the planner is built and called changed. The planner itself
should work, but some of these scripts still call it in the wrong way.
Also, lab will not work on the branch. Jendrik, can you look into this and see
what kind of changes would be required to support the new scripts?

> Supported options are 
> option(WITH_MERGE_AND_SHRINK OFF)
> option(WITH_LANDMARKS OFF)
> option(WITH_PDB OFF)

In case it's not obvious, I should point out that there are far more pluggable
modules at the moment. Essentially, each line of the form

HEADERS += ...

in the Makefile is a pluggable module. For example, cg_cache.h and
cg_heuristic.h form one module, hm_heuristic.h forms one, lmcut_heuristic.h
forms one, etc.

There are some hidden dependencies there (e.g. the FF heuristic depends on
h^add), but not many.

There are further pluggable modules hidden within the "main" modules at the top
of the Makefile. Our ICAPS 2013 tutorial included a list of which files are
actually required to build the planner, but it's no longer up to date. More
clearly separating out the planner core from the optional bits is one of the
things I'd like to find time for over the next year.

Here is the current state of my shot at this:

https://bitbucket.org/flogo/downward-issues-issue67/branch/issue67#diff

It handles modules and the LP stuff, but I have not tried it on other platforms
yet. It is also out of date, i.e. we would need to merge in the current default
branch and update the file lists.

As Malte said in msg3873, we should design a new solution. I don't have much
time to work on this right now, but maybe we can meet some time next week to
discuss this?

About cmake and scons: I'm new to both systems but from looking at the
documentation it seems to me that we could build a nicer build system with scons
since it more directly supports modules with dependencies. However, I also think
that scons will be more work to set up because it is more basic, i.e. more
things have to be programmed instead of being built in. The main why I still
prefer cmake over scons is that cmake can generate project files for all common
IDEs, so everyone can use their favourite IDE without additional setup.

Here is my hacky cmake build system.
https://bitbucket.org/TheOneRing/fast-downward-cmake
It takes -DFD_DIR as an argument and is capable to build process, downward, and val
val with the core configuration.
No support for coin and lp.

Supported options are 
option(WITH_MERGE_AND_SHRINK OFF)
option(WITH_LANDMARKS OFF)
option(WITH_PDB OFF)

I created it be able to create native builds without msys and to create project fi
files for an IDE during my thesis.

I'm quite a fan of cmake because, if done right, it is the best build system I 
have seen so far.

I think it can get more complicated than setting the -m32 option. We also need
to look for the correct version of libraries (e.g., cplex, coin). I ran into
trouble with cmake there on my first try, but I think its possible to do this
cleanly with cmake.

> One thing to look out for might be how difficult it is to do cross compilation
> (e.g., 32 bit on 64 bit system). I don't know Scons, but the following
> stackoverflow answer mentions this as a weak point.

I think they mean "complicated" kinds of cross-compilation where you are
building for an entirely different kind of target system and need to set up the
compiler in complex ways. Configuring a single option like -m32 or -m64
shouldn't be an issue for any build system. (I've used SCons before, but it's
more than 10 years ago, but I think it just means setting some value in a Python
dictionary.)

One thing to look out for might be how difficult it is to do cross compilation
(e.g., 32 bit on 64 bit system). I don't know Scons, but the following
stackoverflow answer mentions this as a weak point. I can look into this if I
find some time.

http://stackoverflow.com/questions/4071880/autotools-vs-cmake-vs-scons#18291580

Personally, I would prefer cmake because cmake projects can be loaded into most
IDEs without additional setup.

I agree that having a Python-based build system would be a plus. Maybe also 
https://code.google.com/p/fabricate/ is worth checking out. Fabricate is very 
lightweight, follows the principle "explicit is better than implicit" and has 
automatic dependency tracking. I have just browsed through its documentation 
though and haven't tested it, yet.

I don't want to derail the plot, but I've been hearing more good things about
SCons again (e.g. ESR's opinions on SCons vs. cmake on
http://esr.ibiblio.org/?p=1877), and it might be easier to get SCons to play
nicely with the plug-in architecture of the planner (cf.
http://parts.stage.tigris.org/).

As far as I recall, we haven't yet considered how easy or difficult it is to
make our plug-in model work with cmake. If it turns out to be not so easy, SCons
might be worth another look.

When working on this, we should ideally also address the possibility of
concurrently configuring 32-bit and 64-bit LP library paths (issue504).

Resetting the status to chatting because it is not really in "reviewing" mode,
right? (It it is, please set back and let us know what and where to review.)

> Should I write an email to downward-dev (or even the main mailing list) to ask
> around which build modes are currently in use, or if anyone would like to see a
> new build mode?

No strong opinions on this; there are arguments for starting with a concept
concocted without too many meddling cooks, and there are arguments for shopping
for opinions. Whatever you prefer.

If we don't specify anything in cmake, it will search for dynamic and static
versions of a library and use whichever is discovered first. Its actually
somewhere between painful and impossible to force it to discover only one type
of library. I think the default way this is set up, is to look for an "*.so"
file first and an "*.a" file second, but this can be changed. If there is no
particular reason to switch between dynamic and static, I would just leave the
choice up to cmake. This should then also work on OS X.

Should I write an email to downward-dev (or even the main mailing list) to ask
around which build modes are currently in use, or if anyone would like to see a
new build mode?

> Ok, so should we start collecting the configurations we want to have?

Sounds good! :-)

I agree with (1) and (2), except that I'm not sure if the reasons why we
originally made static linking the default apply any more (this was mostly about
compiling on machine A and then running on machine B), so we might want to have
a discussion about whether we want static or dynamic builds. But I think that's
a detail we can leave aside for now. (Right now I think that we don't need to
offer a static vs. dynamic option, but I'm not really sure of that, and I'm even
less sure which of the two is the better one.)

Regarding (3) for gprof, if I'm the only one that uses it, I wouldn't much mind
dropping support. I'm sure I'll be able to find a workaround, or if not, add
another build configuration later. Anyone else interested in keeping gprof
support here?

Regarding (4), we will need 64-bit support when running the planner with memory
limits significantly larger than 4 GB. It's not unlikely that we'll want to run
such experiments in the foreseeable future, and I know that others using our
code are already doing this. So my initial opinion, without having thought about
this too deeply, is that it may be a good idea to support a 32-bit and 64-bit
version of everything.

As far as I recall, the main reason why we have a "native" bitwidth option and
allow selecting between dynamic and static linking was for OS X support. So I
don't think these bitwidth and linking options are desirable in themselves.
Rather, I hope we will be able to have OS X support more easily with different
means within our regular build targets. After all, the "C" in "CMake" stands for
"cross-platform". :-)

Ok, so should we start collecting the configurations we want to have?

From my point of view those are:

 (1) Our default release version running on the grid for experiments (-m32,
-static, -O3, -NDEBUG)
 (2) A debug version of (1) (-m32, -static, -O3, -g)
 (3) A profiling version of (1) that can be run with gprof (I don't actually use
this, but from the comments below, it seems like you do)
 (4?) From the comments in the old Makefile it seems that we introduced "-m64"
for the IPC, do we still need this?
 (5?) Does anyone use the DOWNWARD_BITWIDTH=native option or dynamic linking?

All of the above should work both with and without LP solvers installed. I think
it is not necessary that the user can decide whether to use an LP solver or not.
Instead, we should just check which solvers are installed and use all of them.

I think that we are still talking about details too much and that this is not
what we should be doing at this point. For example, I don't know you mean by "an
option".

> So we have 18 potential build directories? (Fine with me, I just want to check).
>
> Bitwidth: 32/64/native
> Mode: release/debug/profile
> Linking: dynamic/static

No, unless we think that this is the best possible design. Let me reiterate: the
new solution should be designed with no regard to how the current solution is
implemented.

For example, if I forget about the existence of our current Makefile, I would
never say we need three different settings for "bit width". Also, do we really
want all these profile builds? Is anyone actually using them? The problem we
want to solve here is not to replicate our existing Makefile in CMake. Rather,
we want to come up with a good build solution that serves our use cases.

> - I'm not a fan of the MixedCase and Capitalized directory names everywhere. It
> feels like moving from Linux to Windows.

We can change the capitalization of the build directories but the cmake names
are all mixed case by default (CMakeFileList.txt, FindCplex.cmake, etc.)

> - From what I've seen of CMake and related tools, changing the toolchain and
> clearing the cache to get a build with a different bitwidth doesn't sound like
> the idiomatic way of doing things. I think the idiomatic way would be to have
> separate build directories for different bitwidths.

For cmake, I think there is not really a standard way to handle bit width. I
agree that having one directory for each setting is better than clearing the
cache, but I think that tool chain files are a good way to handle compiling for
32bit on a 64 bit machine. Some paths and settings are automatically detected
when starting a new project in cmake and they depend on the tool chain. Its a
bit hard to find documentation about this. An interesting thread was on the
cmake mailing list: (http://www.cmake.org/pipermail/cmake/2013-March/054160.html).

On the positive side, an option would reduce the complexity of the cmake call.
On the negative side, it would make the main cmake code more complex and the
option would appear as a changeable option in ccmake. This might be confusing
because changing the option after the first run would result in strange behaviour.

I'm ok with using an option, but we should document how it can be changed.

> - From what I understand, the default build under your suggestion would be
> different from our current default in terms of the mode of linking (dynamic
> instead of static) and in terms of the architecture (32 bit instead of 64 bit).

We can set both a default tool chain file and a default build type, so we can
have any default we want.

> We retested this recently (only this month, I think): a 64-bit compile still is
> very inefficient in terms of memory usage, and we generally try to set our
> defaults so that people who just run the planner for their experiments without
> reading all the fine details of the documentation get decent performance. Hence
> I think it would be good for the default to remain a static 32-bit build, and I
> think the build directories should be more explicit about what they are, e.g.
> "StaticRelease" => "release-32-static" and "release-64-static"; "Release" =>
> "release-32-dynamic" and "release-64-dynamic".

So we have 18 potential build directories? (Fine with me, I just want to check).

Bitwidth: 32/64/native
Mode: release/debug/profile
Linking: dynamic/static

> (Of course, any given users would be likely to only produce one or two of these
> builds: one flavour of release build and perhaps one flavour of debug build.)

I'm not certain yet how these directories are created. Do we just tell the users
to create them and call cmake with the appropriate parameters, or is there a
better way? Maybe build_all can create them?

Please think of these variables as internal parts of the current Makefile, not
as some kind of overarching architectural aspects of the intended build process.
If we switch to a different build solution, all of this will be replaced
completely by something completely different. The new solution should be
designed with no regard to how the current solution is implemented.

I also suggest we get rid of some environment variables:

1) DOWNWARD_USE_LP should be switched to a cmake option and does not need the
prefix DOWNWARD_ anymore. We could even remove the option altogether and just
compile with all solvers available through OSI. The cmake file detects which of
the solvers are available anyway.

2) Instead of having DOWNWARD_CPLEX_INCDIR and DOWNWARD_CPLEX_LIBDIR as
environment variables, I suggest to have only DOWNWARD_CPLEX_ROOT (analogously
for other solvers). If this variable is set, the cmake module searching for
cplex should only search within this directory. If it is not set, the module
should search in the regular library paths.

3) DOWNWARD_OS is not necessary with cmake. The current OS is detected by cmake
anyway, and changing it (i.e. cross-compiling) should be done with tool chain files.

Thanks for looking into this! Some preliminary comments:

- I'm not a fan of the MixedCase and Capitalized directory names everywhere. It
feels like moving from Linux to Windows.

- From what I've seen of CMake and related tools, changing the toolchain and
clearing the cache to get a build with a different bitwidth doesn't sound like
the idiomatic way of doing things. I think the idiomatic way would be to have
separate build directories for different bitwidths.

- From what I understand, the default build under your suggestion would be
different from our current default in terms of the mode of linking (dynamic
instead of static) and in terms of the architecture (32 bit instead of 64 bit).
We retested this recently (only this month, I think): a 64-bit compile still is
very inefficient in terms of memory usage, and we generally try to set our
defaults so that people who just run the planner for their experiments without
reading all the fine details of the documentation get decent performance. Hence
I think it would be good for the default to remain a static 32-bit build, and I
think the build directories should be more explicit about what they are, e.g.
"StaticRelease" => "release-32-static" and "release-64-static"; "Release" =>
"release-32-dynamic" and "release-64-dynamic".

(Of course, any given users would be likely to only produce one or two of these
builds: one flavour of release build and perhaps one flavour of debug build.)

I played around with this for a while. I'm not completely done upgrading this to
the new master branch but I already ran into a couple of problems:
We currently have some options that change the version of a library that should
be used (DOWNWARD_BITWIDTH and DOWNWARD_LINK_RELEASE_STATICALLY). The approach
of cmake is to find the correct library and use its full path. This path is
cached, so running cmake again with different options does not change them. The
only way to change them in this setting is to remove the whole cache. This is
counter-intuitive, e.g. if someone uses ccmake (cmake GUI) to change which
plugins are enabled, then these settings are reset, too.

The "cmake way" to handle this seems to be to either use different tool chains
or build in different directories for the different options. Tool chains can be
configured with a separate file which is only read in with the first call to
cmake. Other calls to cmake can change the plugin options without affecting the
tool chain settings. Before calling cmake with a different tool chain file, the
cache has to be cleared completely. 

I suggest to handle this by using tool chain files for cross-compilation and
different directories for different build modes. I also suggest to add the build
mode "StaticRelease". This would mean that we get the following directory structure:

src/search
  -- CMakeLists.txt (main cmake script)
  -- cmake
  ---- modules (additional cmake scripts)
  -- build
  ---- Release
  ------ CMakeCache.txt
  ------ Makefile (generated by cmake)
  ------ (other temporary cmake files)
  ---- StaticRelease
  ---- Debug
  ---- Profile

The debug build can be generated with "cd build/Release; cmake ../..
-DCMAKE_BUILD_TYPE=Debug". The build type defaults to Release, so "cd
build/Release; cmake ../.." is sufficient to create a Makefile. Cross-compiling
for different tool chains (32 bit on a 64 bit system or 64 bit on a 32 bit
system) works by removing the cache and then re-creating it with the correct
tool chain:

  cd build/Release
  rm -rf *
  cmake ../.. -DCMAKE_BUILD_TYPE=Debug
-DCMAKE_TOOLCHAIN_FILE=../../cmake/modules/Linux-i686-32bit-toolchain.cmake

Once the Makefile is created, plugins can be enabled/disabled with "cmake ../..
-DPLUGIN_XY=ON" without the need to set the tool chain or build mode again (or
with the ccmake GUI). Calling "make" in a directory creates the binary with the
current tool chain and build mode.

Sure, go ahead. Assign this to you if you like. You could also talk to Patrick 
von Reth who currently maintains his own CMake setup.

I would like to look into this issue for selfish reasons (an IDE I'd like to try
only supports cmake builds). Since this has been around for over 2 years,
Jendrik's branch is very much out of date. My first step would be to update it
and merge in the current default branch (or should I rather re-create the branch
based on the current default version?)

Jendrik, is this fine with you?

Thanks for looking into this! I feel like compilation speed is not the big issue
-- we're not close to the size of Chromium. ;-) An incremental compile in ~4
seconds (dividing your numbers by three since we won't usually build three
executable versions at once) is actually quite fine, and if we were worried
about speed, we could make the Makefile much faster by running the dependency
analysis only on modified files rather than on all of them all the time. I'd be
surprised if we couldn't get close to the speed of cmake with a project of our
size, actually.

So to me the more important issues are readability and maintainability of the
Makefile for us, and (maybe to a somewhat lesser extent) ease of use for the
"end users". CMake seems to do very well on these metrics, too, though. :-)

I can't promise a timely review, unfortunately. :-( Anyone else got some time to
spare?

Instructions for testing the cmake build:

hg clone http://bitbucket.org/jendrikseipp/downward
cd downward/src/search
hg up issue67
cmake .
make -j2

To use different options, use e.g.
cmake . -DCMAKE_BUILD_TYPE=DEBUG -DPLUGIN_LANDMARKS=FALSE

I have done quite a bit of research on this and it seems CMake is indeed the best option for us. 

- SCons is deemed too slow for big projects and not very cutomizable.
- The new kid on the block seems to be tup, which is very fast due to an intelligent algorithm for 
detecting which files need to be rebuilt. Unfortunately it invents its own syntax and is not easily 
installable nor is it very extensible. Additionally my experiments have shown that it's hard to 
build hierarchical projects with it.
- Although I don't like its syntax CMake is fast and widely used. The underlying Make system is old 
and slow, but CMake version >= 2.8.8 (available in Ubuntu 12.10) can output ninja files instead of 
Makefiles.
- Ninja is the tool used to build chromium. People say it's very fast, much faster than make. The 
authors say its input files should not be written by hand and instead a tool such as CMake should 
be used for this. I haven't tried it out yet, but will definitely do so once I upgrade to the new 
Ubuntu. The nice thing about this is: Every developer can choose the backend he/she wants.

I have already integrated Andrew's work into the current Fast Downward version and implemented the 
latest feature additions to the Makefile for CMake. All previous Makefile features should work 
(multiple targets, different executables for different state sizes, cross-compiling dor 64-bit, 
easy on/off-switching for the LP stuff) and in addition we have a plugin-system, dependency-
tracking and faster builds. Here are some timing results for compiling the default target (three 
executables) incrementally:

touch globals.cc; ./build_all -j4

make:
real	0m13.997s
user	0m12.097s
sys	0m1.168s

cmake: 
real	0m2.714s
user	0m6.748s
sys	0m0.576s

touch globals.h; ./build_all -j4

make:
real	1m46.616s
user	6m17.636s
sys	0m15.185s

cmake:
real	1m18.475s
user	4m50.970s
sys	0m12.197s

touch merge_and_shrink/shrink_fh.h; ./build_all -j4

make:
real	0m16.464s
user	0m30.382s
sys	0m1.800s

cmake:
real	0m7.969s
user	0m23.493s
sys	0m0.956s

touch merge_and_shrink/shrink_fh.h; ./build_all -j1

make:
real	0m31.946s
user	0m29.202s
sys	0m1.732s

cmake:
real	0m20.835s
user	0m19.545s
sys	0m0.952s

As noted above those times might even improve further with ninja as a backend. Anyway this makes 
our edit->compile->run cycle much faster.

The code is ready to be merged, I opened a code review at http://codereview.appspot.com/6569063 and 
am happy about comments.

I've used SCons quite a bit in the past, and it was always quite a bit of work
to customize the builders to get them to do what I wanted. CMake also has the
advantage that it can produce Makefiles, so that our users don't have to install
more special software to compile our code. (Or maybe SCons can do this too these
days? It's been a while since I used it.)

I'm not against using SCons in principle, but I'd need to be convinced that it's
a better option for us than CMake. The best thing would be to be able to compare
the respective Makefiles/recipes directly and see which ones we like better.

A friend of mine just recommended SCons (http://www.scons.org/) to me. He tells 
me that it makes complete builds as fast as CMake and make (this is of course 
expected) and outperforms the other two when it comes to partial builds. It uses 
Python as its language, so it's more extensible than the other two options as 
well and we don't need to learn another language. Also there's just one step 
involved for building as SCons will not generate any intermediate Makefiles for 
example. Before comitting to CMake, I think we should take a closer look at 
SCons. I will do that when time allows.

It looks like I don't have any cycles to spare on this at the moment. :-(

This is a bit trickier than it looks since it means we'll have to update the
docs, buildbot, new_scripts, etc., and also get our technician to install cmake
on some additional machines.

We should get around to doing this eventually, but it may take a while.

Okay, the attached .tar.gz contains the files you need:

* search/CMakeLists.txt    - the main cmake file itself
* search/cmake/modules/... - a few modules to find the Osi includes/libraries
* search/lp/Makefile       - used by cmake to make sure the supplied Osi sources
are compiled (if needed)
* search/lp/setup          - a tweaked version of the existing script, in which
the shared libraries are disabled

I've tried to write helpful comments in CMakeLists.txt so I hope it's easy
enough to follow.

Quick-start guide for using it:

cd <path containing the search directory>
tar zxvf search-cmake-files.tar.gz

mkdir Release
cd Release
cmake -DCMAKE_BUILD_TYPE=Release ../search

.. and from then on, just:

make

If you want a more verbose make process, to check what compiler flags are being
used:

cmake -DCMAKE_VERBOSE_Makefile=TRUE -DCMAKE_BUILD_TYPE=Release ../search

If you want to enable the LP stuff (building Osi in the source directory, if a
compiled copy is not already there):

cmake -DUSE_LP=TRUE -DCMAKE_BUILD_TYPE=Release ../search

If you want to enable the LP stuff, but use system libraries rather than those
from the sources in the SVN repository:

cmake -DUSE_LP=TRUE -DUSE_LP_INTERNAL=FALSE -DCMAKE_BUILD_TYPE=Release ../search

By default, four build types are defined:
* -DCMAKE_BUILD_TYPE=Debug         - no optimisation, full debugging information
* -DCMAKE_BUILD_TYPE=Release       - -O3 optimisation, static binary, no
debugging information
* -DCMAKE_BUILD_TYPE=RelWithDeInfo - -O3 optimisation, static binary, but with
debugging information
* -DCMAKE_BUILD_TYPE=Profile       - -O3 optimisation, profile information linked in

If you want to turn plugins on and off, you probably want to use one of the
CMake GUIs.  A console based one is ccmake:

ccmake -DCMAKE_BUILD_TYPE=Release ../search

Press 'c' to configure, edit the various 'PLUGIN_...' options to suit, 'c' to
configure again, 'g' to generate.

The main omission for now, apart from not tracking plugin--plugin dependencies,
is that it doesn't have the make profile target.  For now, write yourself a bash
script; later, cmake can generate it if the Profile build type is used.

I think I prefer the second solution, but for now we don't really have any
serious dependencies between plugins, so in the spirit of KISS and YAGNI, I
suggest the following three-step process:

1. Start from a cmake solution that doesn't track any plugin dependencies at
all. Plugins need to be manually enabled/disabled, and it's the users
responsibility to make sure that whatever combination of things they chose makes
sense together.

2. Work with that for a while to see if it's good enough, and if not, where the
problems with this are.

3. Once we're sure we want something more fancy, go with one of your solutions
(e.g. the second one).

Other opinions/comments?

Okay, I've been looking into this, trying to determine whether any existing
plugin-rich projects have a solution out there already.  The answer is, it
appears not - the granularity is usually pretty coarse, with any heavily
inter-dependent plugins being wrapped up into one homogenous lump.

Anyhow, on the basis of getting this right once and for all (hopefully), I'm
going to propose going all-out and going for the plugin dependency tracking at
cmake time option, rather than leaving it to compile/link time.  There are a few
options, with varying degrees of elegance.  The two that I've shortlisted are as
follows:

1) The easiest to maintain is to use (abuse?) CMake's existing dependency
checking.  This would be done in two steps:

i) At cmake time, generate 'stub' header files in the build directory, depending
on the plugins to be compiled.  E.g. specifying that plugin foo should be built
would generate <visible/plugin_foo.h> containing:

#include "plugin_foo.h"

ii) Elsewhere, never ever refer to plugin_foo.h directly - always do

#include <visible/plugin_foo.h>

As this file exists iff the plugin foo is going to be built, then the dependency
tracking will catch cases where plugin bar is referring to foo, but foo is
disabled.  cmake will report an error like:

"Cannot find source file "visible/plugin_foo.h" for target libpluginbar.a"

It's exactly the mechanism it uses to complain about missing source files (e.g.
where they weren't committed to the SVN), but now complaining about files it
didn't auto-generate (because the user neglected to specify that foo should be
built).  The error message also seems to make some sort of sense - so long as
the visible/header.h filenames are sensibly named, it indicates what needs
linking in.

2) Little config files that explicitly record plugin--plugin dependencies, and a
python script ran by cmake time that would take a list of plugins to enable, and
spit out the result of doing a reachability analysis in the plugin dependency
graph.  Then, anything needed would be included without a hitch (maybe printing
out a warning that something is being implicitly included).

Advantages compared to (1)

- auto-fixing errors, with one run of cmake setting everything up, rather than
just complaining at the user until they've specified exactly everything they need
- no need to change #include statements - the desired effect is achieved with no
source changes

Disadvantages:

- the plugin config files (defining what other plugins they depend on) would
need maintaining manually; rather than forcing this to be done by being rigorous
about only ever including header files from <visible/...>.
- python at build time, though this is more-or-less negligible given the
translator is written in python, and python is ubiquitous


I'm happy to hack out either - traversing a digraph in python is about as much
effort as writing a cmake module.  So, your code-base, your call.

Best, Andrew

Sounds good! And yes, it would be great if you could look into this.

About the profile, if this isn't generated with "make PROFILE" but in some other
way, this would be fine as long as it's still easy to do. Just having a bash
script lying around that does the equivalent of "make downward-profile;
./downward-profile ARGS; gprof ..." would do the job.

One point:

> * dependency tracking - again, easy:
> 
> IF (HEURISTIC_FOO_ENABLED OR SOME_HEURISTIC_THAT_DEPENDS_ON_FOO_ENABLED)
> ...also build the object files for FOO
> ENDIF (...)

Ideally, I'd like the dependency tracking between plugins to be explicit in the
sense that if plugin FOO depends on plugin BAR, the developer of plugin FOO has
to specify the dependency. I think what you write above works along those lines.

But here's an additional requirement that might be a bit harder: if plugin FOO
includes a header from plugin BAR and this dependency is *not* specified in the
Makefile (or recipe, or whatever cmake calls it), then I want to get an error,
even if the current configuration would compile (e.g. because it includes both
FOO and BAR, or neither). So there should be a check that all plugin
dependencies (in so far as they can be automatically detected) are indeed
specified. (This is a nice-to-have, not a must-have, but I thought I'd mention
it up front in case it affects the design of the Makefile.)

I'll take this one on if you like, can probably fit it in a spare hour.  Looking
at your list of must-haves:

* LP stuff: easily done.  Pass -DUSE_LP=TRUE to cmake.  For the internals, I
already have cmake scripts from our planners to find the installed files for
Clp, Osi, etc. and complain if the various parts are not found.

* plugins: again, fine.  One can either pass options to the command line to
change the build config, or use the GUI to change which plugins are enabled,
looking something like:

http://www.cmake.org/cmake/img/ccmake.png

* three targets: pass -DCMAKE_BUILD_TYPE=RELEASE, -DCMAKE_BUILD_TYPE=DEBUG or
-DCMAKE_BUILD_TYPE=PROFILE, or ....  It's just a case of noting which compiler
options to use for each.

* generation of profiles: hmm, this is currently done with a target in the
makefile.  The easiest way I can think of porting this is to have cmake
auto-generate a bash script which can then be ran to do the same job.

* dependency tracking - again, easy:

IF (HEURISTIC_FOO_ENABLED OR SOME_HEURISTIC_THAT_DEPENDS_ON_FOO_ENABLED)
...also build the object files for FOO
ENDIF (...)

This one has been languishing for a while. Does someone want to look into this?

Requirements for a CMake solution:

Must-haves: what our Makefile currently does
 * Switching on/off the LP stuff easily.
 * Switching on/off the "plugin" parts of the codebase easily.
 * Supporting our three targets (default, debug, profile) and the generation
   of profiles.

Nice-to-haves:
 * Tracking dependencies between plugins (which we can't currently do).

OK, I had a bit of a look around and watched this Google Tech Talk on
CMake/CTest/CDash/CPack: http://www.youtube.com/watch?v=8Ut9o4OdSC0

Unless I hear of any objections soon, let's switch to cmake.

Okay, the 30-second guide to cmake is:

1) The CMakeLists.txt file lists what needs to be compiled.  Running cmake turns
that into a Makefile.  A CMakeLists.txt file can be as simple as:

add_executable(planner foo.cc bar.cc)

  That will then build the executable planner from foo.cc and bar.cc

2) Arguments to cmake (e.g. -DCMAKE_BUILD_TYPE=Debug) control the build type
(the argument set passed to the compiler).

3) Build dependencies can also be checked by cmake.  In the VAL CMakeLists.txt
file we write:

find_package(BISON)
find_package(FLEX)

For colin we also use:

find_package(Clp)

4) cmake also automates a few other useful things.  For instance, to help track
regressions, it can build a header file called svnversion.h containing a #define
of the current repository revision.

Interesting. I know very little about cmake, but in some way or other, some
update to the build process is overdue. At the very least, the Makefiles would
need to be updated.

This needs discussion with the others.

As the search binary is now built from 71 .cc files, the Makefile.depend step of
the current build system takes upwards of ten seconds each time due to the lag
of accessing files over the network.  Okay, we could blame the file servers over
here, but to work around it I've migrated over to using CMake to handle the make
process.

The CMake configuration file that gives equivalent compilation results to the
current make system is attached.  If you save it to the search directory, then
type (from there):

mkdir Debug
cd Debug
cmake -DCMAKE_BUILD_TYPE=Debug ..
make

..that will produce a Debug build of the planner, called search, within the
Debug directory just created.  Substituting Release for Debug throughout does a
release build (static, as before), and Profile does a profiled build.  As you
may have noticed, it does an out-of-source build - the Debug directory can be
called anything and go anywhere, with the .. argument to the cmake command
pointing to where the sources are.

With reference to issue61 (having more directories), CMake would be ideal for
this - put a CMakeLists.txt in each subdirectory, listing the files to be
compiled from there, and include each of these at the top.