OK, I added a note in issue156 and will close this issue now.

We have 150 open issues, so I suggest we close this issue rather than marking it
as deferred. If you want to keep track of it, we can add a note about this to a
more general landmark code refactoring issue.

(Reasoning behind the recommendation: there are literally dozens of similar
issues with the landmark code; if we open separate issues for all of them with
no plan to ever work on the landmark refactoring, I think we will just lose
track of them.)

That's true, this change may be too intrusive and we should wait until the 
landmarks code is refactored.

> What I meant by "override" is the following: if we have an lm_rhw factory with
> disjunctive_landmarks=true in an lm_merged factory with
disjunctive_landmarks=false,
> then the resulting heuristic will not use disjunctive landmarks. Is that
correct...

It sounds plausible to me that this is what happens with the current code, but
it's convoluted enough that I don't have much confidence in this. But the way I
read the code, it's not overriding in the sense that the lm_merged setting
always "wins". Rather, both get a chance to discard the landmarks. If the
situation where the other way around, with lm_rhw using the false setting and
lm_merged using the true setting, I think we'd end up without disjunctive
landmarks, too.

> ...and the behaviour you would expect?

As written in my previous message, it would make sense for lm_merged not to have
any of these options in the first place. This may be an intrusive change in an
already brittle codebase, though; I'm not sure it ranks in the top 100 of
problems with the current landmark code. ;-)

What I meant by "override" is the following: if we have an lm_rhw factory with disjunctive_landmarks=true in 
an lm_merged factory with disjunctive_landmarks=false, then the resulting heuristic will not use disjunctive 
landmarks. Is that correct and the behaviour you would expect?

> Can't we just add the relevant options to each factory?

That would be nice.

> If I understand the code correctly, the options passed to lm_merged override the
> options of the individual landmark factories.

Are you sure about this? That's not what I would expect. Maybe I'm
misinterpreting the word "override".

> If we can set all options in the individual factories, does lm_merged need any
> options at all?

It would make sense for lm_merged to just take whatever its "child factories"
have generated, yes.


Generally speaking, the landmark code is a mess in dire need of refactoring.

Currently, each landmark factory has the options reasonable_orders, 
only_causal_landmarks, disjunctive_landmarks, conjunctive_landmarks and 
no_orders, although only a subset of them is relevant for each factory. Can't we 
just add the relevant options to each factory?

If I understand the code correctly, the options passed to lm_merged override the 
options of the individual landmark factories. If we can set all options in the 
individual factories, does lm_merged need any options at all?