Issue775

Title driver: add --validate-memory-limit and --validate-time-limit options; also handle memory and time limits separately
Priority feature Status resolved
Superseder Nosy List jendrik, malte, salome, silvan
Assigned To silvan Keywords driver
Optional summary

Created on 2018-04-10.12:13:09 by jendrik, last changed by silvan.

Messages
msg7748 (view) Author: silvan Date: 2018-09-21.15:46:27
Thanks, I merged the issue and sent an email to our group.
msg7747 (view) Author: jendrik Date: 2018-09-21.15:43:02
Sounds good to me. I merged the Lab branch.
msg7745 (view) Author: silvan Date: 2018-09-21.14:07:40
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.
msg7740 (view) Author: salome Date: 2018-09-21.13:55:14
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 :).
msg7739 (view) Author: malte Date: 2018-09-21.13:38:30
That should be fine.
msg7737 (view) Author: silvan Date: 2018-09-21.13:37:20
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.
msg7735 (view) Author: malte Date: 2018-09-21.13:22:13
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).
msg7732 (view) Author: jendrik Date: 2018-09-21.13:02:28
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.
msg7726 (view) Author: malte Date: 2018-09-21.12:44:07
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?
msg7725 (view) Author: silvan Date: 2018-09-21.12:43:25
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?
msg7724 (view) Author: jendrik Date: 2018-09-21.12:41:14
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.
msg7715 (view) Author: jendrik Date: 2018-09-21.12:07:52
I left one trivial comment but this looks good to be merged.
msg7713 (view) Author: jendrik Date: 2018-09-21.12:03:20
I think tracking this in the issue tracker would be good. I'll have a quick look 
at the diff as a whole again.
msg7712 (view) Author: malte Date: 2018-09-21.12:01:25
No strong preference. Getting Augusto involved in this sounds good.
msg7711 (view) Author: silvan Date: 2018-09-21.12:00:08
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.
msg7710 (view) Author: malte Date: 2018-09-21.11:56:46
Sounds good to me! Silvan, is the reviewing done after Jendrik's comments?
msg7709 (view) Author: jendrik Date: 2018-09-21.11:53:30
Yes, I commented on the individual commits.
msg7708 (view) Author: silvan Date: 2018-09-21.11:50:19
Just saw that Jendrik already commented; didn't get emails.
msg7707 (view) Author: silvan Date: 2018-09-21.11:49:09
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.
msg7695 (view) Author: silvan Date: 2018-09-21.09:18:59
I changed the code so that val's limits are not part of --overall-*-limit anymore.
msg7690 (view) Author: jendrik Date: 2018-09-21.01:22:45
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.
msg7653 (view) Author: silvan Date: 2018-09-20.16:43:42
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?
msg7612 (view) Author: jendrik Date: 2018-09-19.21:14:32
I left some comments on Bitbucket.
msg7595 (view) Author: malte Date: 2018-09-19.18:22:38
Sounds good! No objections to merging from my side then.
msg7593 (view) Author: silvan Date: 2018-09-19.17:28:46
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.
msg7586 (view) Author: malte Date: 2018-09-19.16:22:32
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.
msg7579 (view) Author: silvan Date: 2018-09-19.16:02:11
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
msg7575 (view) Author: silvan Date: 2018-09-19.15:09:01
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.
msg7027 (view) Author: jendrik Date: 2018-04-10.12:30:06
We should probably wait for issue739 to be merged before working on this.
msg7026 (view) Author: jendrik Date: 2018-04-10.12:13:09
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.
History
Date User Action Args
2018-09-21 15:46:27silvansetstatus: reviewing -> resolved
messages: + msg7748
2018-09-21 15:43:02jendriksetmessages: + msg7747
2018-09-21 14:07:40silvansetmessages: + msg7745
2018-09-21 13:55:14salomesetmessages: + msg7740
2018-09-21 13:38:30maltesetmessages: + msg7739
2018-09-21 13:37:20silvansetmessages: + msg7737
2018-09-21 13:22:13maltesetmessages: + msg7735
2018-09-21 13:02:28jendriksetmessages: + msg7732
2018-09-21 12:44:07maltesetmessages: + msg7726
2018-09-21 12:43:25silvansetmessages: + msg7725
2018-09-21 12:41:14jendriksetmessages: + msg7724
2018-09-21 12:07:52jendriksetmessages: + msg7715
2018-09-21 12:03:20jendriksetmessages: + msg7713
2018-09-21 12:01:25maltesetmessages: + msg7712
2018-09-21 12:00:08silvansetmessages: + msg7711
2018-09-21 11:56:46maltesetmessages: + msg7710
2018-09-21 11:53:30jendriksetmessages: + msg7709
2018-09-21 11:50:19silvansetmessages: + msg7708
2018-09-21 11:49:09silvansetmessages: + msg7707
2018-09-21 09:18:59silvansetmessages: + msg7695
2018-09-21 01:22:45jendriksetmessages: + msg7690
2018-09-20 16:43:42silvansetmessages: + msg7653
2018-09-19 21:14:32jendriksetmessages: + msg7612
2018-09-19 18:22:38maltesetmessages: + msg7595
2018-09-19 17:28:46silvansetstatus: in-progress -> reviewing
messages: + msg7593
2018-09-19 16:22:32maltesetmessages: + msg7586
2018-09-19 16:02:11silvansetmessages: + msg7579
title: driver: add --validate-memory-limit and --validate-time-limit options -> driver: add --validate-memory-limit and --validate-time-limit options; also handle memory and time limits separately
2018-09-19 15:09:01silvansetstatus: chatting -> in-progress
assignedto: silvan
messages: + msg7575
nosy: + silvan
summary: Blocked by issue739. ->
2018-04-10 12:30:06jendriksetstatus: unread -> chatting
messages: + msg7027
summary: Blocked by issue739.
2018-04-10 12:13:09jendrikcreate