issue17

issue17

[malte.helmert, 2014-04-21 12:13:22]

https://codereview.appspot.com/89740043/diff/1/misc/make_ipc_submission
File misc/make_ipc_submission (right):

https://codereview.appspot.com/89740043/diff/1/misc/make_ipc_submission#newcode9
misc/make_ipc_submission:9: #SANITY_TEST_RUN_TEST1=1
I would enable all tests by default and rename the last one to "..._TEST_RUN" if
there's only one. But it's probably better to have two tests instead with
separate options since many planner versions won't be able to solve the PSR task
(which requires axioms).

https://codereview.appspot.com/89740043/diff/1/misc/make_ipc_submission#newco...
misc/make_ipc_submission:73: DIR="$(cd "$(dirname "${BASH_SOURCE[0]}" )" &&
pwd)"
Isn't "$0" clearer and more correct than "${BASH_SOURCE[0]}"?

https://codereview.appspot.com/89740043/diff/1/misc/make_ipc_submission#newco...
misc/make_ipc_submission:82: rm -f output.sas output sas_plan
We used to verify that the plan matches a given known plan. If we don't do this
any more, we should at least run the validator. (Make sure that it is run on the
correct plan files in case we're using an anytime planner.)

https://codereview.appspot.com/89740043/diff/1/misc/make_ipc_submission#newco...
misc/make_ipc_submission:92: ./build
Doesn't the patch remove this script?

[jendrikseipp, 2014-04-21 17:04:04]

I incorporated your comments. Will upload a new patch.

https://codereview.appspot.com/89740043/diff/1/misc/make_ipc_submission
File misc/make_ipc_submission (right):

https://codereview.appspot.com/89740043/diff/1/misc/make_ipc_submission#newcode9
misc/make_ipc_submission:9: #SANITY_TEST_RUN_TEST1=1
On 2014/04/21 12:13:22, malte.helmert wrote:
> I would enable all tests by default and rename the last one to "..._TEST_RUN"
if
> there's only one. But it's probably better to have two tests instead with
> separate options since many planner versions won't be able to solve the PSR
task
> (which requires axioms).

Done.

https://codereview.appspot.com/89740043/diff/1/misc/make_ipc_submission#newco...
misc/make_ipc_submission:73: DIR="$(cd "$(dirname "${BASH_SOURCE[0]}" )" &&
pwd)"
On 2014/04/21 12:13:22, malte.helmert wrote:
> Isn't "$0" clearer and more correct than "${BASH_SOURCE[0]}"?

Done.

https://codereview.appspot.com/89740043/diff/1/misc/make_ipc_submission#newco...
misc/make_ipc_submission:82: rm -f output.sas output sas_plan
On 2014/04/21 12:13:22, malte.helmert wrote:
> We used to verify that the plan matches a given known plan. If we don't do
this
> any more, we should at least run the validator. (Make sure that it is run on
the
> correct plan files in case we're using an anytime planner.)

Done.

https://codereview.appspot.com/89740043/diff/1/misc/make_ipc_submission#newco...
misc/make_ipc_submission:92: ./build
On 2014/04/21 12:13:22, malte.helmert wrote:
> Doesn't the patch remove this script?

The "build" file is written by the make_ipc_submission script now.

[malte.helmert, 2014-04-21 18:19:24]

https://codereview.appspot.com/89740043/diff/20001/misc/make_ipc_submission
File misc/make_ipc_submission (right):

https://codereview.appspot.com/89740043/diff/20001/misc/make_ipc_submission#n...
misc/make_ipc_submission:10: SANITY_TEST_RUN_ADL=0
Derived predicates aren't an ADL feature. I'd rather call it
SANITY_TEST_RUN_AXIOMS or similar.