Clarify use of TaskIndex

[gfrances]

Hi @miquelramirez , I am trying to understand and fix / improve the TaskIndex class, which has changed from the lite branch to the dev-2.0 branch, and is breaking some of my code. In particular, assume that index is a TaskIndex... when I invoke index.process_symbols(problem), now index.fluent_symbols no longer contains symbols, but FormulaReferences. Is there any reason for this? My understanding is the following: a symbol is either fluent or static, depending on whether its denotation changes over any state in the problem. fluent_symbols used to collect which symbols in the signature of my language were fluent, and for that we don't need TermReferences or FormulaReferences (of course, we can also speak of fluent atoms and fluent terms; interestingly, not every atom formed from a fluent predicate symbol will necessarily be fluent, as it could be that a reachability analysis detects that it is static over some particular instance).

At the same time, in visitors.py a comment reads:

Visitor method to sort atoms and terms into the
"fluent" and "static" categories. Note that a given
symbol can be in both sets, this means that it gets
"votes" as static and fluent... the post_process() method
is meant to settle the issue (and potentially allow for
more ellaborate/clever heuristics).

which makes me think that I am missing something. What are the cases in which a symbol can be both fluent and static? Tried to look for the post_process() method, but doesn't seem to exist anymore.

What are the general use cases of TaskIndex in your code? Are you using it just to compute state variables? Or any other thing?

[miquelramirez]

Is there any reason for this?

To make the list searchable, that's why we need to wrap them. That is useful to implement efficiently compilations and avoid duplicate entries.

What are the cases in which a symbol can be both fluent and static?

That is a comment that makes reference to an attempt to implement directly real axioms (or ramifications) as state constraints. So the comment is pretty out of date.

What are the general use cases of TaskIndex in your code? Are you using it just to compute state variables? 

Basically to discern between state variables and constants, yes. It follows the same functions as the FS Python helper class TaskIndex, but rather than depending on assumptions implicit on the type of input text file (e.g. PDDL as used in the IPC benchmarks), how symbols are treated is made explicit in the code by means of visitors, etc.

[gfrances]

Ok! My point was that before we used to have predicate symbols in that attribute, and now we have atoms, which is not the same, but then I suggest to have two different sets of methods / attributes: fluent_symbols and static_symbols, on the one hand, and fluent_terms and static_terms, on the other (although this last option ignores the fact that they might be atoms as well, but I don't have a better alternative in mind...). The first set will hold, for instance in Gripper, a list with Python objects:

static_symbols = [Predicate(symbol='room'), Predicate(symbol='ball'), ...]
fluent_symbols = [Predicate(symbol='at-robby'), Predicate(symbol='at'), ...]

whereas the second set should hold what it is holding now, e.g.

static_terms = [FormulaReference(phi=room(roomA)), FormulaReference(phi=room(roomB)),, ...]
fluent_terms = [FormulaReference(phi=at-robby(roomA)), FormulaReference(phi=at(ball1, roomB)),, ...]

What do you think?

[miquelramirez]

Hi @gfrances,

I am okay with changing the name of the attributes to "xxx_terms" rather than the current arrangement. I will do the change myself in dev-0.2.0 and then synchronise my projects.

[miquelramirez]

See commit 4cbaaf99f8eb383a1b6460780ef4fe9faf5fc47f for the renaming/introduction of the new attributes. All tests are passing locally for me.

[gfrances]

Ok, I see that I was wrong in my previous description (but you didn't correct me!): fluent_terms still seems to be aimed at computing fluent "symbol" objects, not terms. Brrrr. But still: I don't understand what is all of this different, repeated FormulaReferences that we are computing. First point: I don't think we need "reference wrappers" around simple logical symbols (i.e. Functions or Predicates); hash and equality are properly defined for them. Second point: I'm not sure the computations done by task index are correct. See e.g. the commit efdcd6e in branch dev-0.2: I added a test with a sample gripper instance which is exactly the same instance that is giving me a lot of troubles when trying to switch my projects to this branch. The predicate at-robby, for instance, is identified both as static and fluent; and several instances of each predicate make it into the sets, with different arguments (but: why would we want to keep the treat different argument names as distinct?).

If you could clarify a bit for me the expected outcome of the TaskIndex in e.g. this simple gripper task, I could try to help seeing if it needs to be fixed or not, and eventually fixing it, if necesary?

[miquelramirez]

Hi @gfrances,

I hope you guys enjoyed your tropical island holidays. Let me go over the items above below

Necessity of wrapping Function and Predicate objects

Totally agreed. I am not sure what is the objection though to wrap atoms with FormulaReference instances when we put them in lists. I do not know either why you think I want to wrap Function and Predicate objects either.

Classifying atoms and terms as either statics "symbols" or fluent "symbols"

The strategy at a high level is to consider all terms and atoms static, unless we find some "evidence" to the contrary. That is, the expected behaviour should be that:

1. We visit the AST of every constraint and precondition and mark every compound, non-built in term and every atom of non-builtin predicates as static "symbols". I resist referring to those as "static fluents", but I am happy to call them something else than "symbols".

2. We then visit the AST of every effect and mark terms and atoms as "fluents" according to rules specific to every type of effect (i.e. functional vs logical).

3. The list of static terms and atoms is then defined as: Statics = Statics \ Fluents. You can find that on line 30 of task_index.py.

On the necessity of keeping instances of the same Predicate and Function

The reason for keeping several instances is that we may found somebody writing

P(?x, a)

but never writing

P(?x, ?y)

in his effects or preconditions. Why should we generate ground atoms and terms for bindings of ?y other than a. Not currently implemented, but part of the plan, was to make the grounding aware of any constraints relevant to the specific variables used.

[miquelramirez]

If you could clarify a bit for me the expected outcome of the TaskIndex in e.g. this simple gripper task, I could try to help seeing if it needs to be fixed or not, and eventually fixing it, if necesary?

What is missing in the current implementation is a way to determine that two atoms (with different arguments) need to be considered for grounding separatedly or not. This is an issue as we generate several times the same grounding. But note as well that the current grounding module (see tarski.grounding.naive) is called naive for a reason. It is meant to be a baseline, that provides very basic functionality not performance.

The reason I haven't looked further into this is that I don't need efficient grounding at the moment and I was kind of expecting you to detail further what you had in mind w.r.t. grounding in general (see issues #15 and #16).

[gfrances]

I think this has already been dealt with with the integration / refactoring of the ASP based grounder. Feel free to reopen if you think otherwise, of course!