Issue1029

Title Unclear error message when "define" keyword is missing
Priority feature Status in-progress
Superseder Nosy List jendrik, malte, victor.paleologue
Assigned To Keywords
Optional summary
When submitting a wrong PDDL domain lacking the "define" statement to Fast Downward, the translator gives very little information about what is wrong.

See issue220.

Created on 2021-07-19.08:10:33 by victor.paleologue, last changed by victor.paleologue.

Summary
When submitting a wrong PDDL domain lacking the "define" statement to Fast Downward, the translator gives very little information about what is wrong.

See issue220.
Messages
msg10433 (view) Author: victor.paleologue Date: 2021-09-01.14:27:12
Thanks for explaining. I'll remove the change log entry, since this change is so minor. Thus we rule out the communication issue.

For the legal issue, there is nothing I can do to help. I thought having also the MIT-like license and being owner of the brand gave you that security.

What would be your final conclusion on this PR, then?
https://github.com/aibasel/downward/pull/59
msg10432 (view) Author: malte Date: 2021-09-01.14:16:05
It is not the integration process as such, it is more that we generally like to approach related improvements in larger chunks for communication reasons. A changelog entry is also a communication to the user base. There are currently probably at least 20 ways in which the error reporting for invalid PDDL should be improved, and we'd rather communicate this to the users in one or a few communications of the form "We finally improved the handling of PDDL. Hope you like the change!" rather than with 20 separate changelog entries.

There's also the question of what accepting externally contributed pull requests like these means for the contributors file and the legal status of the project. Even for GPL-ed projects like these, authorship is important. For example, currently in research proposals sent to public funding agencies, we can say "We're bringing into the project the Fast Downward planner, for which we hold the rights", and this is something that helps us get funding for our research. Once we don't own the planner any more, that becomes problematic. (GPL is all well and good, but for example for EU funding proposals there is a huge bureaucratic process where legal teams need to be kept happy. It's not great, but it's the way it is.)

Things like these can of course often be arranged, but it's more worth the effort for cases where it comes with substantial progress.
msg10426 (view) Author: victor.paleologue Date: 2021-09-01.09:36:39
Hi Malte,
I understand that stance, however let us get into the specifics.
The PR https://github.com/aibasel/downward/pull/59 just adds a message to an existing assertion. The code is there, though you there is something to fix there, the issue is there, the changelog is there. What is it in your integration process that would really cost so much that compromises accepting the PR?
msg10423 (view) Author: malte Date: 2021-08-31.12:47:37
Hi Victor, this is an improvement in the right direction, but every change, even a small one, takes some work to integrate, and hence I'd prefer to improve the error reporting in somewhat larger steps than this. Assertions are not really the correct way to report invalid inputs, we're only using them as a crutch until we have something better. I would prefer to move on directly to something that we'd be happier with in the medium term rather than to a better version of a stop-gap measure.

BTW, the pull request violates our style guide regarding line lengths.
msg10420 (view) Author: malte Date: 2021-08-30.19:51:32
See issue220.
msg10419 (view) Author: malte Date: 2021-08-30.19:49:51
Hi Victor, thanks for reporting! This is parting of a more general problem that PDDL error reporting is currently very poor. I'll try to have a look when I have some cycles to spare, but not sure if this will happen in the near future. :-( If this sits idle for several months from now, feel free to ping me (by answering here on the tracker or by email).
msg10366 (view) Author: victor.paleologue Date: 2021-07-20.10:16:11
New corrected PR: https://github.com/aibasel/downward/pull/59
msg10365 (view) Author: victor.paleologue Date: 2021-07-19.08:10:32
Related PR: https://github.com/aibasel/downward/pull/43
History
Date User Action Args
2021-09-01 14:27:13victor.paleologuesetmessages: + msg10433
2021-09-01 14:16:05maltesetmessages: + msg10432
2021-09-01 09:36:39victor.paleologuesetmessages: + msg10426
2021-08-31 12:47:38maltesetmessages: + msg10423
2021-08-30 19:51:52maltesetassignedto: victor.paleologue ->
2021-08-30 19:51:32maltesetmessages: + msg10420
summary: When submitting a wrong PDDL domain lacking the "define" statement to Fast Downward, the translator gives very little information about what is wrong. -> When submitting a wrong PDDL domain lacking the "define" statement to Fast Downward, the translator gives very little information about what is wrong. See issue220.
2021-08-30 19:49:51maltesetnosy: + malte, jendrik
messages: + msg10419
2021-07-20 10:18:36victor.paleologuesetmessages: - msg10367
2021-07-20 10:18:25victor.paleologuesetmessages: + msg10367
2021-07-20 10:16:11victor.paleologuesetmessages: + msg10366
2021-07-19 08:10:33victor.paleologuecreate