Bug with grounding of atoms detected as static by reachability analysis

[gfrances]

This bug arises in the same situation as the one described in issue #113. The last action of the parcprinter domain:

(:action sys-Stack-Letter
 :parameters ( ?sheet - sheet_t ?prevsheet - sheet_t)
 :precondition (and
        [...]
        (Location ?prevsheet Some_Finisher_Tray)
        [...])

features a precondition atom (Location ?prevsheet Some_Finisher_Tray). The ASP grounder (correctly) detects as a possible grounding of the action the tuple (sheet1, dummy_sheet). At the same time, it (also correctly) detects that (Location dummy_sheet Some_Finisher_Tray) is not a state variable, since, according to the reachability analysis, one can infer that its value will never change: the atom holds in the initial state, and one can see that there is no action that deletes atoms of the form (Location X Some_Finisher_Tray).

Now, the problem comes when the C++ grounder takes the valid action-grounding (sheet1, dummy_sheet) and tries to instantiate the atom with that grounding. Because it detects that in (Location dummy_sheet Some_Finisher_Tray) both arguments are constants, it infers that the resulting atom can be "converted" into a state variable, since Location is a fluent symbol. The lookup for the corresponding state variable however fails and throws a runtime error, since as we saw above, it is indeed not a state variable.

This will in general happen as long as we have a fluent symbol paired with a reachability analysis that detects as static some of its possible instantiations. The backend is not ready at the moment to deal with this granularity in fluency detection - it assumes that all atoms P(X1, ..., XN) resulting from a fluent symbol P are state variables, if X1, ..., XN are constant symbols, or fluent-headed symbols - but we actually do not want either of those, we want to detect the condition as satisfiable in preprocessing-time and act accordingly.

A hacky workaround btw would be to force this type of atoms to be considered state variables, even though they value will never change.

[miquelramirez]

Hello @gfrances,

I think that the so-called optimisations due to static fluent removal are getting in the way a bit. For a radical thought, what about disabling static detection and optimisation for all but atoms where the predicate has a fixed denotation? For instance in action schemas like

(:action foo 
     :parameters (?x - object ?y - object)
     :precondition (not (= ?x ?y))
     :effect (and (bar ?x) (not (bar ?y)) )

the precondition could be certainly compiled away.

This could be as difficult as changing the heuristic we have in the front end to identify static atoms.

I am doubtful that we're going to see a "massive" impact in performance if we make more conservative "optimisations".

[miquelramirez]

By the way, I just recalled that I added a command line flag to our runner script to disable static atom analysis, see runner.py line 50. It may be worth to see how much we're losing by having the extra ground actions.

[gfrances]

Yes, this type of simple rewritings as the one in the example you post is also handled at the C++ level (i.e. when not using the ASP parser). The ASP parser is more powerful than the handcoded approach we have at the C++ level, in the sense that it detects atoms that are static for a certain particular instance (i.e. given an initial state + reachability analysis). I used to have some reachability analysis at the C++ level but rarely use it these days, it is most likely not working anymore :-).

This type of optimization certainly does not make a huge difference in most of the IPC benchmarks. So yes, an option would be to ignore it...

[gfrances]

By the way, I just recalled that I added a command line flag to our runner script to disable static atom analysis, see runner.py line 50. It may be worth to see how much we're losing by having the extra ground actions.

Looks like you mean that all possible atoms are considered as fluents? Mmmm not sure what's the gain with that? That would mean having much larger states in many problems - and, OTOH, atoms derived from static symbols pose no problem at all for us, so why would we need to deal with them as if they were fluents?

[miquelramirez]

Let me rephrase that.

The option I added was necessary for an online planner to work correctly... I wanted the set of state variables to be fully exposed so the executive could set the values of states to match the state of the simulation of the environment, when there is no model available to account for the evolution of those variables over time. That can be a matter of convenience, practicality or necessity (if say, those values are trying to account for noisy actuators, etc.).

What I propose is to get that a bit tighter: just consider static those atoms whose denotation is fixed, that is, built-in predicates like equality. You end up with bigger states - maybe an issue on purely propositional representations. On multi-valued ones, having an extra literal in the domain of a variable should not be a big deal.

[gfrances]

just consider static those atoms whose denotation is fixed, that is, built-in predicates like equality.

Mmm not sure I follow now... What is the difference between static and fixed? I tend to use them as synonims. Does your definicion include symbols such e.g. connected in visitall?

[miquelramirez]

Fixed: like equality, you shouldn't have an action setting the atom (= 2 2) to true, or false. The meaning of those predicates is something given and standardised. Static: something contingent on the particular initial state given.

The case connected is interesting: there we're reifying the characteristic function of a set whose definition remains constant, if there's no action removing or adding elements to the set. Or rather, if there is no action detected as "relaxed reachable" that does so, from the given initial state. That could depend on whether a given atom is true in the initial state.

[gfrances]

ok ok, but there's a difference between static or fixed atoms and static or fixed symbols, that's where my misunderstanding was coming from. A symbol is static = fixed in principle if its denotation will no change through the execution of any plan. A symbol might be fluent, but some of the atoms derived from it might be static (this is where the problem in this issue is coming from). But if a symbol is static, e.g. because no effect in the domain changes its denotation (e.g. connected), then there is little point in factoring it inside of the state, at least in classical, propositional planning. So all in all I think we're on the same page here.

[gfrances]

This same issue also happens in the Woodworking domain, just for the record. In instance p01, available(p0) is not listed as a state variable by the ASP preprocessor, but available is a fluent symbol, hence the backend fails. It is easy to see the reason of this: available(p0) is initially true, and there is no delete effect in any action affecting the symbol available.

[gfrances]

Again for the record, it turns out that if I disable the clever ASP fine-grained reachability-based static atom detection, then we solve no tetris atom at all. This makes sense, as even the smaller instance goes from having a few hundred fluent atoms to having around 70K atoms - and we mem-out quite fast. Maybe we'll have to solve the issue after all, although that looks like quite an amount of work: there's probably many places in the code where it is assumed that if a symbol is fluent, then all the atoms derived from it are fluent as well...

[miquelramirez]

Thanks for the observation @gfrances - just remember that the strictures of prop languages are relevant until March 6th. After that...

[ssardina]

Hi @miquelramirez and @gfrances .

Seems you are after a big shark bug :-) So I am not sure this is helpful but I suspect it is related to the --disable-static-analysis, which may not be working. If I use that option with walkbot I get:

./run.py --hybrid --disable-static-analysis --instance examples/hybrid/continuous/walkbot/instance_001.pddl --domain examples/hybrid/continuous/walkbot/domain.pddl --driver sbfws --options dt=0.5,integrator=runge_kutta_2,bfws.rs=sim,width.force_generic_evaluator=true,bfws.using_feature_set=true,features.project_away_numeric=true,features.project_away_time=true,features.elliptical_2d=true,width.simulation=2,sim.log=true

[INFO][ 0.00298] Number of objects: 9
[INFO][ 0.00300] Number of state variables: 13
[INFO][ 0.00300] Number of problem atoms: 6
[INFO][ 0.00300] Number of action schemata: 12
[INFO][ 0.00301] Number of (perhaps partially) ground actions: 0
[INFO][ 0.00301] Number of goal atoms: 8
[INFO][ 0.00302] Number of state constraint atoms: 4
[INFO][ 0.00312] Deriving control to search engine...
terminate called after throwing an instance of 'std::runtime_error'
  what():  spx::LinearProgramm::add_constraint() : At the moment the right hand side of a constraint needs to be a constant
Peak memory: 67004 KB
caught signal 6 -- exiting
Aborted
...

I guess you know about this already?

[gfrances]

So you mean that if you don't disable the static analysis, everything works fine, but if you disable it you get that runtime error? Miquel is definitely your man here, but that sounds like some other minor bug?

[miquelramirez]

@ssardina that isn't an error but the expected behaviour. If you disable the statics it can happen that was used to be a linear constraint, ceases to be linear or conform to the pattern the Lp compiler expects when you disable the optimisation for statics.

[ssardina]

Yes @gfrances , when I do not disable it, it works perfectly.

But now @miquelramirez argues this is expected as the constraints you generate are not linear anymore. I guess this may be due to some technicalities, because if you compiled away only static functions, then linear constraints should stay linear right? But there must be some technicalities in the translation that breaks the linearity..

[gfrances]

Ok, I finally and unfortunately found out that the quick fix that I applied to fix the above problem (i.e. the one described in the beginning of this thread) is not really a fix, since it fails in many corner cases. This means that as of now, using the --asp flag can result in a potentially incomplete planner because of the abovementioned reasons. In particular, and just for (my) record, the quick fix fails because it fixes which atoms are considered state variables and which not, but does not make sure that all those that are considered state variables are given a value in the initial state. This is because this information is already lost after going through the ASP grounder, which has discarded them as static.