Issue1055

Title Translator ignores predicates unused by actions, even if mentioned in initial state and goal and differ
Priority bug Status resolved
Superseder Nosy List florian, jendrik, malte, mkatz, patfer, silvan
Assigned To patfer Keywords
Optional summary
PR: https://github.com/aibasel/downward/pull/102
Translator times: https://ai.dmi.unibas.ch/_tmp_files/ferber/issue1055-v1-issue1055-base-issue1055-v1-compare.html
Changed output.sas files: https://ai.dmi.unibas.ch/_tmp_files/ferber/issue1055-different-output.csv

Created on 2022-04-08.16:32:07 by mkatz, last changed by patfer.

Summary
PR: https://github.com/aibasel/downward/pull/102
Translator times: https://ai.dmi.unibas.ch/_tmp_files/ferber/issue1055-v1-issue1055-base-issue1055-v1-compare.html
Changed output.sas files: https://ai.dmi.unibas.ch/_tmp_files/ferber/issue1055-different-output.csv
Files
File name Uploaded Type Edit Remove
issue1055.diff malte, 2022-04-21.20:11:04 text/x-patch
small_example.tar.gz mkatz, 2022-04-08.16:32:07 application/x-gzip
Messages
msg10716 (view) Author: malte Date: 2022-05-09.16:04:17
Thank you! I left two comments on the change log entry. Once they are addressed, please merge.
msg10715 (view) Author: patfer Date: 2022-05-09.15:54:11
All checks are passing now and a changelog is added.

If you are happy, I merge it.
msg10714 (view) Author: malte Date: 2022-05-06.19:07:30
Thanks a lot! I think the github actions fail because I included two blank lines at the end of instantiate.py. Can you remove those?

The difference in the translator for the trucks-strips instance is known; this is because there is a timeout on the invariant synthesis, and depending on how fast/slowly the run goes, it will find slightly different invariants.

For citycar it's a bit more concerning, although I think we've seen it before. But I suppose we can treat it as unrelated if we also get different outputs for citycar with the current code. If you really want to be on the safe side, we could we run a quick before/after comparison on this domain only for lama-first to make sure that there aren't issue-related problematic changes in addition to the existing nondeterminism.

But I'm also OK if you think we don't need to run this test. Then the last thing is a change log entry. Should I write it or do you want to?

I agree that the hack is not that bad, and in particular it's better than the previous hack that it replaces.
msg10713 (view) Author: patfer Date: 2022-05-06.07:57:01
I applied your diff and set up the experiments. Here is the PR: https://github.com/aibasel/downward/pull/102

The time looks almost the same:
https://ai.dmi.unibas.ch/_tmp_files/ferber/issue1055-v1-issue1055-base-issue1055-v1-compare.html
On a first glance looks like noise. Do you want a scatter plot with one data point per instance?

But is the output correct? The following CSV file lists instances where the generated output.sas file differs. The first column indicates the reason why the task is listed.
- N failed: N configurations failed to generated the output.sas file. This is ok as long as N = 2 (all)
- N differ: N different output.sas files were generated. This is problematic and happens for some
-- citycar
-- trucks
instances. I executed the current main branch multiple times on the first citycar instance and also got multiple different output.sas files within seconds. Didn’t we removed non-determinism at some point (except for hitting resource limits)
 https://ai.dmi.unibas.ch/_tmp_files/ferber/issue1055-different-output.csv


PS: In you patch you mark a block of code as HACK! Resolving this looks at first as adding a class Goal to the pddl module with almost no content and looks a bit overkill. If you want we can do this, but I would keep your HACK, as it is not that complicated.
msg10706 (view) Author: malte Date: 2022-04-22.10:16:39
PS: If someone else continues this, if you give me push access to your clone, I can push an issue branch with the diff I posted. I currently have this in a local clone on my home desktop because I haven't set up my own Fast Downward clone on github yet.
msg10705 (view) Author: malte Date: 2022-04-21.20:11:04
I started looking into this. The problem is in instantiate.py, which resolves static atoms occurring in actions or axioms, but not in the goal. I don't have a suitable setup at the moment to integrate this properly, so I'm attaching a tentative patch for now as a starting point (so far without a change log entry). Perhaps someone else can turn this into a proper issue branch and do some tests? 

Ah, I just notice this was assigned to Patrick, which I overlooked before. @Patrick: sorry if I duplicated some work you perhaps already did. Perhaps you can take it from here?

In addition to testing the example provided by Michael, we should test that the translator output isn't otherwise affected and test the effect on runtime. This particularly needs testing on non-STRIPS domains.
msg10695 (view) Author: silvan Date: 2022-04-11.09:54:40
(Moving Michael's original message to the change note field.)

Translator ignores predicates unused by actions, even if mentioned in initial 
state and goal and differ. This results in a solvable sas problem, while the PDDL 
is not solvable.
Small example is added.
History
Date User Action Args
2022-05-09 16:34:02patfersetstatus: in-progress -> resolved
2022-05-09 16:04:17maltesetmessages: + msg10716
2022-05-09 15:54:11patfersetmessages: + msg10715
summary: PR: https://github.com/aibasel/downward/pull/102 Translator times: https://ai.dmi.unibas.ch/_tmp_files/ferber/issue1055-v1-issue1055-base-issue1055-v1-compare.html Changed output.sas files: https://ai.dmi.unibas.ch/_tmp_files/ferber/issue1055-different-output.csv
2022-05-06 19:07:30maltesetmessages: + msg10714
2022-05-06 07:57:01patfersetmessages: + msg10713
2022-04-22 10:16:39maltesetmessages: + msg10706
2022-04-21 20:11:04maltesetfiles: + issue1055.diff
messages: + msg10705
2022-04-11 09:54:40silvansetmessages: + msg10695
summary: Translator ignores predicates unused by actions, even if mentioned in initial state and goal and differ. This results in a solvable sas problem, while the PDDL is not solvable. Small example is added. -> (no value)
2022-04-11 09:00:11patfersetstatus: unread -> in-progress
nosy: + malte, jendrik, silvan, florian, patfer
assignedto: patfer
2022-04-08 16:33:50mkatzsetnosy: + mkatz
2022-04-08 16:32:07mkatzcreate