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

Aggregate Semantic Check Missing #128

Closed
zyzek opened this issue May 27, 2016 · 4 comments
Closed

Aggregate Semantic Check Missing #128

zyzek opened this issue May 27, 2016 · 4 comments
Labels
bug - identified Bugs with an identified cause
Milestone

Comments

@zyzek
Copy link
Collaborator

zyzek commented May 27, 2016

Program crashes while writing data to a file.

segfault_count_sccs.zip

@zyzek zyzek added the bug - identified Bugs with an identified cause label May 27, 2016
@zyzek zyzek added this to the 1.0 milestone May 27, 2016
@b-scholz
Copy link
Member

The compiled version of Souffle works; however, the interpreter seg-faults in visitStore() in the RamExecutor.cpp.

@b-scholz b-scholz changed the title Bug storing relation. Aggregate Semantic Check Missing May 30, 2016
@b-scholz
Copy link
Member

I have further investigated the issue. The problem with that example is the following line

scc(A, C) :- C = min D : {mutually_reaching(A,B), id(B, D)}.

The issue is that here we assume that we not just compute the minimum value of the set

mutually_reaching(A,B), id(B, D)

but also request the witness environment for the minimum value. In this case, the witness environment would constitute of values for variables A, B, and D at the time when the
minimum was observed. The witness issue could be semantically justified for min/max. However, for counting, it would make no sense.

scc(A, C) :- C = count : {mutually_reaching(A,B), id(B, D)}.

There is no single value of A that could produce a sensible value for the relation scc.

@b-scholz
Copy link
Member

I forgot to say because we don't have a semantic check that stops witnesses (and we don't store them at the moment), the above program produces either invalid results for variable A or crashes converting the invalid value of A to a symbol.

The easiest solution for the problem would be to check for witnesses and report warnings. Note that a witness is basically an unground variable in the outer scope of the aggregation. We would need a semantic check that prohibits unground variables in the outer scope that become grounded in the aggregation scope.

Alternatively, we permit witnesses for min/max and extend the interpreter and compiler for storing the witness environment. The runtime for aggregation may increase if a witness environment is stored. However, for this solution, we need still need the semantic check for the count-aggregation. For a count-aggregation, witnesses do not make sense and must not be permitted.

I would suggest to enforce the witness check first, i.e., no witnesses are permitted and later add the extension if required by some users.

@ghost
Copy link

ghost commented May 25, 2017

As of pull request #387, there is now a test in tests/evaluation/count_sccs2/count_sccs2.dl for this issue. When this issue is solved, uncomment the line in evaluation.at marked with # TODO (see issue #128) POSITIVE_TEST([count_sccs2],[evaluation]) and update the relevant output files in the test directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug - identified Bugs with an identified cause
Projects
None yet
Development

No branches or pull requests

3 participants