Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixing eatom cache #2

Merged
merged 2 commits into from
Dec 19, 2017
Merged

fixing eatom cache #2

merged 2 commits into from
Dec 19, 2017

Conversation

tobiaskaminski
Copy link
Contributor

The storage dictionary for the eatom evaluation cache is not indexed by interpretations, but by a set containing all input atoms. Because of this, the eatoms are not evaluated again when the interpretation changes. This can be seen with the following program:

d(0). d(1).
a(b) ; n_a(b).
num(X) :- &num[a](X), d(X).

And the corresponding external atom implementation:

import dlvhex

def num(a):
	n = 0
	for x in dlvhex.getInputAtoms():
		if x.tuple()[0] == a and x.isTrue():
			n += 1
	
	dlvhex.output((n, ))
	
def register():
	dlvhex.addAtom("num", (dlvhex.PREDICATE, ), 1)

These are the correct answer sets:

{a(b),d(0),d(1),num(1)}
{n_a(b),d(0),d(1),num(0)}

These are the returned answer sets:

{a(b),d(0),d(1),num(1)}
{n_a(b),d(0),d(1),num(1)}

I propose only using true input atoms to index the storage dict to fix this 6814b97.

@peschue
Copy link
Member

peschue commented Dec 18, 2017

Thank you, you are right, the problem is that all input atoms are used as key, independent of their truth value.

The only problem of your solution: if only true assignments are used, in case of partial assignments there can be issues again. However, currently, only full assignments are used. To avoid unpredictable behavior from future changes, and to document this situation, I suggest that you add an assertion about having a nonpartial assignment before the if:

assert(all(x.isTrue() or x.isFalse() for x in predicateinputatoms))

What do you think about this?

Apart from that, I suggest to construct the frozenset only once, put it into a variable, and then use it in the statements that you edited. This reduces code duplication.

@tobiaskaminski
Copy link
Contributor Author

Thank you, that's right, I hadn't thought about this case. I've pushed the update.

@peschue peschue merged commit 16a3ed8 into hexhex:master Dec 19, 2017
@peschue
Copy link
Member

peschue commented Dec 19, 2017

Thank you, I have merged the PR. I will also create an issue to create a new test case for this situation. (Your PR description would make a very nice test case!)

peschue added a commit that referenced this pull request Mar 16, 2018
see PR #2 and issue #3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants