Issue994

Title Allow cyclic landmark graphs
Priority wish Status resolved
Superseder Nosy List clemens, jendrik, malte, salome, silvan, thomas
Assigned To Keywords
Optional summary
This is part of meta issue987.

Created on 2021-01-26.21:29:06 by thomas, last changed by clemens.

Summary
This is part of meta issue987.
Messages
msg11289 (view) Author: clemens Date: 2023-08-23.14:53:46
I believe this issue is subsumed by issue996. Originally, issue996 aimed to introduce a landmark factory that takes as input another landmark factory and outputs the result of that factory with potentially fewer orderings, ensuring that the outputted graph is acyclic. This would replace a command line option to turn on or off cycles.

However, the objective of issue996 anyways changed to geting rid of the cycle-breaking code entirely. This is mostly because we no longer rely on acyclic graphs for anything. (Previously, LAMA had this requirement, but after refactoring the landmark progression in issue1036 this is no longer a requirement.)

I hereby close this issue because I don't think it is necessary anymore. For reference, I also linked this issue from issue996 to not loose track of the places where cycle breaking happens currently. For what it's worth: these places are already dealt with in the currently open pull request for issue996 and I hope that one will be resolved soon as well.
msg10045 (view) Author: clemens Date: 2021-02-11.18:30:54
This is the only place I found when working on my Master's thesis (besides the make_acyclic function (or whatever it's called). I meant to ask you whether you covered the part you mention in msg10044, but must have forgotten...
msg10044 (view) Author: thomas Date: 2021-02-11.18:26:18
It seems that allowing cyclic landmark graphs is not just a matter of calling LandmarkFactory::mk_acyclic_graph() or not, there are also parts in the code where edges that lead to cycles are never even added. One such example is the "if block" that starts with the comment "simple cycle test" in "LandmarkFactory::edge_add(...). It might be a good idea to collect occurrences like this one here to simplify working on this issue.
msg9828 (view) Author: silvan Date: 2021-01-27.08:10:53
Add link to meta issue.

Before introducing such a command line option, we should first discuss if we actually want such an option or, if alternatively, the code should only make the graph acyclic if this is needed by the chosen configuration.
msg9820 (view) Author: thomas Date: 2021-01-26.21:29:06
The LandmarkFactory class makes every LandmarkGraph acyclic by removing orders in the presence of cycles. However, not every use case requires an acyclic landmark graph.

In this issue, we'd like to introduce a command line option that determines if the LandmarkGraph is made acyclic. The challenge of this issue is to find out if there are parts of the code that require an acyclic landmark graph and to deal with the new command line option appropriately in this case.
History
Date User Action Args
2023-08-23 14:53:46clemenssetstatus: chatting -> resolved
messages: + msg11289
2021-02-11 18:30:54clemenssetmessages: + msg10045
2021-02-11 18:26:18thomassetmessages: + msg10044
2021-01-27 08:10:53silvansetmessages: + msg9828
summary: This is part of meta issue987.
2021-01-26 21:29:06thomascreate