Issue1108

Title Rename landmark cost assignment to cost partitioning.
Priority wish Status resolved
Superseder Nosy List clemens, jendrik, malte, silvan
Assigned To Keywords
Optional summary

Created on 2023-08-31.10:50:35 by clemens, last changed by clemens.

Messages
msg11443 (view) Author: clemens Date: 2023-10-09.13:58:11
I merged yesterday. Thanks for your inputs Jendrik and Malte!
msg11431 (view) Author: clemens Date: 2023-10-05.13:56:44
I changed it to plural, it definitely feels more natural. I also rebased the changes onto the current main branch. With that, it looks to me like we're happy with the changes. Malte also already gave his approval on the pull request (even before renaming the files), so I interpret this as ready to merge. I'll wait a bit before I actually merge, but if nobody objects until the end of the week, I'll go ahead and do it. Experiments should not be necessary as there are no semantic changes, only renaming of functions, variables and files.
msg11424 (view) Author: malte Date: 2023-10-04.19:35:42
We don't have to bikeshed this if you want to get it done, but I'd prefer the plural over the singular, as it's multiple algorithms. (I don't think the comparison to "assignment" helps that much, apart from the fact that we want to improve on the old names, because these terms are not really on the same level.)
msg11423 (view) Author: clemens Date: 2023-10-04.18:30:09
Thanks for your take on the "uniform" discussion. I agree that it might be better to do this in a separate issue because there doesn't seem to be an obvious candidate name.

I now also renamed the "landmark_cost_assignment.(h|cc)" files to "landmark_cost_partitioning_algorithm.(h|cc)". I was wondering whether "algorithms" (plural) would be better because the file includes multiple implementations, but singular is at least analogous to how it was before (i.e., assignment). Another option would be to split each file into multiple files so that every algorithm has its own file. I don't have a strong opinion. Leaving it the way it is may lead to increasingly large files if we add more cost partitioning algorithms in the future. However, splitting them apart would result in a blow up of the number of files in the landmarks directory, which I also somewhat dislike. But I'm fine with either solution.
msg11421 (view) Author: malte Date: 2023-10-04.16:28:58
Approved on github. I think staying with "uniform" for now is a good choice. I can understand those who would prefer the name to be more precise, but I don't think we need to address this within this issue.
msg11419 (view) Author: clemens Date: 2023-10-04.16:23:45
I think I've incorporated all comments from the pull request. Are we happy with the current status? Then I would move on to renaming the files.

There's an open discussion from issue1091, namely that what we currently call uniform cost partitioning is actually some hybrid of uniform cost partitioning and opportunistic uniform cost partitioning. I don't have a good suggestion how to rename it, so I'm not sure what to do with it. In the bigger context of adding more cost partitioning algorithms (issue800) I definitely think we should entangle this more, so maybe we could also just defer this question until we work on that?
msg11358 (view) Author: jendrik Date: 2023-09-07.16:20:42
I'd use either Algorithm or Method, with a preference for Algorithm. I left some comments on the PR.
msg11357 (view) Author: malte Date: 2023-09-07.15:19:28
I left one comment. I think the new names are an improvement, although they don't click 100% for me.

I looked at https://ai.dmi.unibas.ch/papers/seipp-et-al-icaps2017.pdf, in which I think we primarily use "cost partitioning algorithm" and "cost partitioning method".

Anyone else have thoughts on this? Jendrik, as the main author of this paper, which class names would you recommend?
msg11356 (view) Author: clemens Date: 2023-09-07.15:09:08
I've done some first renamings and opened a pull request to start the discussion: https://github.com/aibasel/downward/pull/178
msg11322 (view) Author: malte Date: 2023-09-04.14:20:54
Updating terminology is good. The code probably predates "cost partitioning" becoming an established term.

We then need to think about what the terminology should be. The most common use of "cost partitioning" is for the actual family/tuple of cost functions. From my memory, our "LandmarkCostAssignment" class isn't itself a cost partitioning; rather, I think it implements an algorithm or multiple algorithms for computing cost partitionings.

When doing a change like this, I would recommend checking the terminology in the immediate vicinity (= in the main files that this code lives in) more generally. For example, I would update the method name "cost_sharing_h_value" at the same time.

Should we start with a pull request and see if we're happy with the suggested names? File renames always make looking at diffs more complicated, so perhaps we can first do everything other than the actual file renaming and then review that version?
msg11301 (view) Author: clemens Date: 2023-08-31.10:50:35
While working on issue1091 we discussed that we might want to update the terminology of landmark cost assignment to cost partitioning. I personally can't recall reading the term cost assignment in the literature and it seems to me that it is simply another term for what we commonly call cost partitioning. That being said, is there a good argument for using "assignment" rather than "partitioning"?
History
Date User Action Args
2023-10-09 13:58:11clemenssetstatus: chatting -> resolved
messages: + msg11443
2023-10-05 13:56:44clemenssetmessages: + msg11431
2023-10-04 19:35:42maltesetmessages: + msg11424
2023-10-04 18:30:09clemenssetmessages: + msg11423
2023-10-04 16:28:58maltesetmessages: + msg11421
2023-10-04 16:23:45clemenssetmessages: + msg11419
2023-09-07 16:20:42jendriksetmessages: + msg11358
2023-09-07 15:19:28maltesetmessages: + msg11357
2023-09-07 15:09:08clemenssetmessages: + msg11356
2023-09-04 14:20:54maltesetmessages: + msg11322
2023-08-31 10:50:35clemenscreate