In issue257 we want to allow overlapping landmarks. Many parts of the code assume that each atom (aka FactPair) occurs in at most one landmark. Landmark factories make sure this is the case (unless we use conjunctive landmarks in lm_hm), but it is an unreasonable restriction on this level. If a heuristic thinks it benefits from having non-overlapping landmarks, it should deal with this on its own, because other heuristics might benefit from overlaps instead (see for example Büchner, Christen and Eriksson, SoCS 2024).
While landmark factories are the main place where this assumption is used, it is not the only place. To make things a little bit easier to digest, I suggest to deal with all places outside the landmark factories first and get rid of this assumption there. After this is done, we can get back to the bigger issue of landmark factories using this assumption.
So this issue wants to reimplement all parts of the code that currently use that at most one landmark exists for a given atom. I've identified two places where this happens:
(1) In landmark heuristics to compute preferred operators, LandmarkHeuristic::generate_preferred_operators(...) to be precise.
(2) In the code for Cartesian abstractions to diversify abstraction collections based on landmarks.
For (1), I already have a suggestion for a work-around. Instead of computing on demand whether a certain fact achieves a future landmark, we precompute which landmarks are achieved by any given fact (could be more than one in theory, but with the assumption underlying the issue it's at most one for now) and checking whether any of them is future during search.
In an experiment, I've compared the difference between the two implementations and it seems minor to me. It includes reference runs where no preferred operators are used and the difference between the two code versions seems to be bigger in these reference runs (even though the implementation makes no difference for those) than in the runs that actually underly changes, so maybe everything is just noise again. In the lama-first-pref runs, there's not much of a change at all, except a slight decrease (~2) in time_scores (maybe due to the precomputation). An even larger decrease (~3) occurs in the reference runs lama-first, though. In contrast, the time_scores improve by ~6 for the lm_zg-pref runs and stay the same in its reference lm_zg. In the lama anytime configuration, the costs are slightly worse over all, but looking at the domain-wise comparison, it is sometimes better and sometimes worse. Again, the reference runs yield a larger decrease in performance regarding the final costs.
Full details can be found here:
https://ai.dmi.unibas.ch/_experiments/buechner/downward/untangle-lm-prefops/data/untangle-lm-prefops-v1-sat-eval/untangle-lm-prefops-v1-sat-3c8a65b5d-a2d652541-compare.html
https://ai.dmi.unibas.ch/_experiments/buechner/downward/untangle-lm-prefops/data/untangle-lm-prefops-v1-anytime-eval/untangle-lm-prefops-v1-anytime-3c8a65b5d-a2d652541-compare.html
Regarding (2), since I'm not really familiar with that code, I've had a brief discussion with Jendrik. It seems that the assumption is not at all important there and it should be easy to get rid of it. I'll try to do so now. @Jendrik: Do you have a suggestion which search configuration I should run to make sure nothing bad happens in this change?
Before I open a pull request, I would be interested to hear whether the results I already have for (1) sound concerning to anybody. If not, and it actually turns out that (2) is easy to fix, I hope we can soon move on from this issue.
|