Thanks, I merged the issue and sent an email to our group.

Sounds good to me. I merged the Lab branch.

Jendrik, we discussed that the decision of weather Lab should always add
--validate or not is not part of this issue. So I suggest that we merge this
one, that you integrate my pull-request for Lab, and that we inform
downward-dev/ai-list that everybody has to use the latest Lab version to ensure
that memory- and time-limits are set.

The problem for me was that some of the tasks I use are not conformant to the
syntax VAL uses (I think some things were declared in the wrong order or
something like that). Since I never actually need VAL in my experiments it was
easiest for me to just disable it. Of couse, one could also fix the pddl files
instead :).

That should be fine.

I think that currently, Lab always calls validate, passing domain, problem, and,
if exists, plan file. Hence if no plan was found, we only validate problem and
domain.

If no plans are produced, the driver should not call validate. This is not
something that should be controlled by options like --validate because also in
our standard experiments, it is possible that no plans are produced for various
reasons (e.g. unsolvable tasks in the standard collection, incomplete planner
configurations, or simply the fact that the planner did not manage to solve the
task because it ran out of time or memory).

I think for example Salomé needs this functionality for unsolvable planning 
tasks. Salomé can you explain why you need this again? I think "validate 
domain.pddl problem.pddl" (without a plan file) should work without errors.

I see this more as a lab issue than something we should address in the driver.
If someone passes --validate --validate-time-limit=0s, they deserve to be
treated poorly.

What is the use case for not validating in lab?

Right. Removing --validate means that we would need to add it to the driver
options of all of our experiments, correct? Alternatively, we would need to
write --validate-time-limit '0s' if we want to disable it, correct? From a
cleanliness perspective, I'd prefer the first option, but from the perspective
of typing/writing this often, I'm less sure... What are your thoughts?

One thing that's still not possible is deactivating the validator in Lab, right? 
What's your preferred solution for this? I see the options of removing --validate 
from the default Lab options or disabling the validator if the corresponding time 
or memory limit is 0.

I left one trivial comment but this looks good to be merged.

I think tracking this in the issue tracker would be good. I'll have a quick look 
at the diff as a whole again.

No strong preference. Getting Augusto involved in this sounds good.

Yes, all done now. Except for that question whether we want to have an issue for
the buildbot things or not and we need to wait for Lab support.

Sounds good to me! Silvan, is the reviewing done after Jendrik's comments?

Yes, I commented on the individual commits.

Just saw that Jendrik already commented; didn't get emails.

I did a few further changes: cleaner separation of limits.py and call.py, now
always exiting if attempting to retrieve or set memory/time limits and that
isn't supported on the used platform. I also fixed the translator to be run on
Windows and added tests for Windows to test-exitcodes.py. 

Unfortunately, the only two things that hinder us from running the full
"run-all-code-tests.py" both on macOS and Windows is that we don't have a
working Python3 installation on the former and cannot build without using "nolp"
on the latter because we have no working CPLEX installation. I think it would be
great if someone (Augusto?) could fix these two things on the two machines
because then we would not need any further special cases for our buildbots.
Since this task is not really about changing Fast Downward, except for the
master.cfg file, I'm not sure if I should open an issue for this.

Apart from that, I would be happy for another round of reviewing and will now
add support for the driver exitcodes and validate limits to Lab.

I changed the code so that val's limits are not part of --overall-*-limit anymore.

Yes, I think we need to add --validate-time-limit and --validate-memory-limit to 
the default driver options in Lab. I'd go with 30m and 3584M to mirror the 
overall limits.

I'm done with addressing or answering them.

Malte had a concern regarding enforcing time/memory limits for VAL: it doesn't
make sense to make this part of the overall limit if we use this for our
experiments with LAB, because we don't want to not run validate because after
search, there are only a few seconds left. So we probably need to adapt Lab to
set a time/memory limit manually. Or do you see any other option, Jendrik?

I left some comments on Bitbucket.

Sounds good! No objections to merging from my side then.

Jendrik said that we are fine if the code respects the overall-limit (which Lab
sets), which it does in my view since I use the same functionality as setting
limits for the translator and search. He will have another look on the code, though.

Superficially, the pull request looks good to me. Does this change the default
validator memory limit from 3.5 GB to something else, or is that the default for
memory limits anyway? I'm mainly asking because we presumably need to set some
kind of limit for experiments on the grid.

Unless I'm missing something, adding the limit was very simple. Here is a
pull-request: https://bitbucket.org/SilvanS/fd-dev/pull-requests/40/issue775/diff

After pushing issue825, we realized that we enforce setting memory (and time)
limits for running validate, which is not possible on mac OS. This issue will
address this problem by distinguishing between time and memory limits in the
driver and not forcing default values for VAL.

We should probably wait for issue739 to be merged before working on this.

We want to make the limits for VAL configurable. 

We still need to find a way to disable validation for Downward Lab experiments. 
Currently, "--validate" is always added to the list of driver options and there 
is no way to unset it. Letting "--validate-time-limit=0" disable validation 
could be one option.