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.

This issue is associated to this GitHub PR: https://github.com/aibasel/downward/pull/59

We merged the pull request for issue1030, which also covers this issue.

I'm marking this as resolved, but more things could be done, as discussed in issue1030.

Thanks for reporting this and for your patience, Victor! I've also added you to the README as a contributor as discussed.

It works for me.

Hi Victor, we'll use the new pull request which doesn't include your commit, but does include the functionality change. We should mention you as a contributor in the README file because you were involved in this change. There's no email addresses recorded in the README file, so if it's understood that this is a personal contribution and not one by the company, we'll be fine. Let us know if this works for you.

Ok, I understand.
I think I can safely rewrite this commit with a personal address if it helps, let me know.

Sorry, that last comment got cut off. What I wanted to add was that the difficulty with copyright (see what is mentioned below) is something that makes merging changes like these difficult in general, especially when they are associated with commercial email addresses.

Hi Victor,

we have had a pull request with completely rewritten error checking for the translator for some time. It supersedes and conflicts with your two pull requests, so we were planning to integrate that one instead as mentioned in my previous message. It has been reviewed and tested, but last time I checked it got stuck due to some (small) unaddressed code review comments. Part of the delay is that the person that worked on it left the group, so there has been some confusion about who will take this to completion.

We'll prioritize landing that patch soon. I'm sorry for your negative experience. As I explained two years ago, 

Best,
Malte

There has always been a pull request for this: https://github.com/aibasel/downward/pull/59

It is just about adding some message to an existing assertion.
It is still applicable.

I don't know where the refactoring is at.
This change is so trivial it's not worth waiting for 2 years to merge it.

If there is something wrong about the PR, I'm not sure I have the patience to deal with it anymore. In that case you can close the PR and the issue, I won't mind.

Hi Victor, I hope that your patch is covered by the much more extensive error checking that we worked on in February, but I am unclear of the status of that work.

To those reading here: is there an issue number for this? Is there a pull request? Is the process stuck, and if yes, can we make it unstuck?

Hello, any news? Is the error handled properly on master now? Should I drop my PR?

Yes, we have been working on the error messages for the last two weeks. Expect some progress soon.

Hello, any news?

Let me repeat what I wrote on the pull request, so the tracker stays up to date: We said that we'll consider pulling something about error checking PDDL in the next sprint in January. If we do, this will also cover this issue, if not, we can merge the current pull request as a stopgap.

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

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.

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?

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.

See issue220.

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).

New corrected PR: https://github.com/aibasel/downward/pull/59

Related PR: https://github.com/aibasel/downward/pull/43