rpgoldman / europa-pso

Automatically exported from code.google.com/p/europa-pso
0 stars 0 forks source link

Planner should not throw exception with inconsistent initial state. #104

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Add these lines to the initial state for any working NDDL model:

int counter;
counter = 0;
counter = 1;
eq(counter, 4);

If you try to run the planner, an exception will be thrown:

ConstraintEngine/base/ConstrainedVariable.cc:59: Error: 
dom.intersects(baseDomain()) is false
        int:CLOSED[1, 1] not intersecting int:CLOSED[0, 0]

The model is ok, and this should be just as acceptable as a model for which the 
planner can't find a solution.

Original issue reported on code.google.com by tristanb...@gmail.com on 4 Mar 2011 at 12:17

GoogleCodeExporter commented 8 years ago
try using specify() instead of =

Original comment by javier.barreiro@gmail.com on 4 Mar 2011 at 12:37

GoogleCodeExporter commented 8 years ago
That does change the behavior - the planner seems to never stop, which also 
surprises me.  

Other variants:

1 int counter = [1];
  counter.specify(2);

This also throws an exception, but a different one.

2.  int counter;
counter.specify(2);
counter.specify(3);

This one finds a solution.  Shouldn't this also be inconsistent?

I don't remember enough details to remember if the latter is a bug.  

However, my point with this ticket is that the planner should never throw an 
exception if you're using legal NDDL syntax!

Original comment by tristanb...@gmail.com on 4 Mar 2011 at 12:50

GoogleCodeExporter commented 8 years ago
yes, I agree this probably needs to be handled better, I was just pointing out 
that the syntax you used may not be what you intended. Also, having valid nddl 
syntax doesn't mean you shouldn't expect exceptions, you can still get 
exceptions that report semantic problems.

== : specifies an equals constraint on a variable
= : sets the base domain for a variable. should only be able to monotonically 
restrict the base domain 
specify() : sets the current domain for a variable, only limitation is that you 
can't get outside of the base domain for the variable (that's why your example 
1 above fails and example 2 works).

I'm never been completely happy with the syntax, I understand it's not obvious 
for the user what's going on. I've thought about removing the ability to 
specify the base domain, but left it for backwards compatibility, it only 
allows for some small performance gains and it complicates the semantics for 
the user.

Original comment by javier.barreiro@gmail.com on 4 Mar 2011 at 1:07

GoogleCodeExporter commented 8 years ago
Now that's exactly the kind of description we should have in our documentation! 
:)

Yes, I didn't completely understand the syntax.  Now that I do, case 1) above 
turns out to be improper syntax and can be ignored, and case 2 makes sense.  

So, I guess the question is whether the planner should be able to handle the 
case where an inconsistent base domain is set:

int counter;
counter = 0;
counter = 1;

Original comment by tristanb...@gmail.com on 4 Mar 2011 at 1:47

GoogleCodeExporter commented 8 years ago
it should throw a semantic  exception in that case, what are you getting?

Original comment by javier.barreiro@gmail.com on 4 Mar 2011 at 1:52

GoogleCodeExporter commented 8 years ago
I think it's what you're expecting:

ConstraintEngine/base/ConstrainedVariable.cc:59: Error: 
dom.intersects(baseDomain()) is false
        int:CLOSED[1, 1] not intersecting int:CLOSED[0, 0]

Should we close this, then?

Original comment by tristanb...@gmail.com on 4 Mar 2011 at 5:20

GoogleCodeExporter commented 8 years ago
if you're getting an exception then we should close it, that's the expected 
behavior.
if you're getting a crash we need to look into why an exception isn't being 
thrown.
I guess the error message could be improved in any case (the raw assertion is 
cryptic to an end user).

Original comment by javier.barreiro@gmail.com on 4 Mar 2011 at 7:17

GoogleCodeExporter commented 8 years ago
also, I just saw a created issue 8 a while ago to deal with this, you're 
welcome to tackle the long term solution if you have the cycles :-)

Original comment by javier.barreiro@gmail.com on 4 Mar 2011 at 7:19

GoogleCodeExporter commented 8 years ago
I agree that issue 8 would be worth tackling if we have time, and think it's ok 
to close this one now.

Original comment by tristanb...@gmail.com on 4 Mar 2011 at 7:24