No more commented out code (at least not relating to this) since r4921.

Reopening -- commenting out functionality is never a complete solution to a
problem. The coupling is still there, it's just not active at the moment.

The lines that are commented out in the Makefile should be removed, not
commented out, and the files should also be removed. Dead code is always
trouble. (BTW, of the three files that are commented out, two could be kept --
only the landmarks one is causing trouble. But maybe you want to remove the
other two anyway since they don't do much.)

Resolved in r4919.

I'll do it early next week (It's the Jewish new year holiday now)

I like Erez's suggestion: get rid of the dependency for now, but open an issue
for more elaborate schemes in the future. I'd suggest doing the minimal change
for that, i.e., just get rid of the LM-related code in the learning directory.

Erez, do you want to do that? I could do it too, no biggie.

In general, it should be alright to have dependencies between components -- e.g.
a plugin could be an additional abstraction strategy for merge&shrink, which of
course would require the merge&shrink component.

It's just that every dependency is of course a cost, and for the LM code in
particular I'd like to have as little dependencies as possible for the near
future because that code is in need of some heavy refactoring.

Regarding Gabi's question what it means for something to be "core": for me,
something is part of the core if it is absolutely essential for compiling and
running the planner. This should be as small as possible. For example, right now
no heuristic is part of the core -- of course you need to enable at least one to
do anything useful, but there is no particular single heuristic that must always
be part of the planner.

What the consequences in terms of namespaces and file locations are is a good
question where I don't have a settled opinion yet. Right now there are no
consequences.

I didn't think about this too much, but probably leave the classifiers,
basic feature extractors, and state space sampling in learning/,
and move the selective max stuff to its own directory.

On Wed, Sep 8, 2010 at 1:00 PM, Gabi Röger <downward.issues@gmail.com> <
downward.issues@googlemail.com> wrote:

>
> Gabi Röger <roeger@informatik.uni-freiburg.de> added the comment:
>
> > Another option is to define the basic learning code (classifier, feature
> > extractor) as part of the "core", and then have the landmarks feature
> extractor
> > defined in the landmarks directory, and have some sort of plugin style
> mechanism
> > for feature extractors as well.
>
> What exactly do you mean with defining it "as part of the core" (in
> terms of file locations, namespaces, etc.)?
>
> _______________________________________________________
> Fast Downward issue tracker <downward.issues@gmail.com>
> <http://issues.fast-downward.org/issue118>
> _______________________________________________________
>

-- 

--------------------------------------------------------------
"Adventure is just bad planning."
    Roald Amundsen
    Norwegian Arctic & Antarctic explorer
    (1872 - 1928)
--------------------------------------------------------------

> Another option is to define the basic learning code (classifier, feature 
> extractor) as part of the "core", and then have the landmarks feature extractor 
> defined in the landmarks directory, and have some sort of plugin style mechanism 
> for feature extractors as well.

What exactly do you mean with defining it "as part of the core" (in
terms of file locations, namespaces, etc.)?

The dependency is there because we have the option of using landmark status as 
features. That option has never really helped anything, so I'm willing to get 
rid of it.
Another option is to define the basic learning code (classifier, feature 
extractor) as part of the "core", and then have the landmarks feature extractor 
defined in the landmarks directory, and have some sort of plugin style mechanism 
for feature extractors as well.

I suggest going for the first option now (getting rid of the dependence), and 
adding the second option as an issue to be done in the future.
Any objections?

In plugin-izing the heuristics (issue117), I noticed that the "learning plugin"
(selective max) fails to compile if the "landmark plugin" is not included since
it uses some parts of the landmark code.

I'd much prefer to have the different components as independent as possible
since this allows much more rigorous refactoring. (In this case, parts of the
landmark code cannot be changed without also changing the learning code.)

One simple possibility would be to rip out the landmark-related code from
selective-max, since at the moment it is unused anyway as far as I can tell.
Erez, do you want to keep it?

If yes, we can of course also think of other solutions.