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

Added limitsize directive #1599

Merged
merged 17 commits into from Aug 28, 2020
Merged

Added limitsize directive #1599

merged 17 commits into from Aug 28, 2020

Conversation

b-scholz
Copy link
Member

This is a very basic implementation for the limitsize directive that stops evaluating a stratum when one or more of the specified relation have reached their specified cardinalities in the limitsize directive. Note that limitsize is a very rough measure and is mainly useful for debugging purposes. This is related to issue #637.

The syntax of the limitsize is

.limitsize A(n=100)

Here is an example,

.decl A(x:number)
A(1).
A(x+1) :- A(x), x< 1000.
.output A
.limitsize A(n=47)

that terminates the evaluation when A reaches the cardinality of 47.

@@ -0,0 +1,79 @@
/*
* Souffle - A Datalog Compiler
* Copyright (c) 2013, 2014, Oracle and/or its affiliates. All rights reserved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update date/organisation.

#pragma once

#include "ram/Expression.h"
#include "ram/NodeMapper.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like all these includes are needed. Remove the unused ones.

@@ -884,6 +885,7 @@ io_head_decl
: INPUT_DECL { $$ = AstIoType::input; }
| OUTPUT_DECL { $$ = AstIoType::output; }
| PRINTSIZE_DECL { $$ = AstIoType::printsize; }
| LIMITSIZE_DECL { $$ = AstIoType::limitsize; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem to be at all related to I/O. Could it instead be a new AST type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Longterm we should rename io-directives to relation-directives.
The reason is that we should not build two different directive infrastructures, i.e., one for IO and the other one for computation like limitsize.

@@ -137,6 +137,18 @@ void ParserDriver::addIO(std::unique_ptr<AstIO> d) {
return;
}
}
} else if (d->getType() == AstIoType::limitsize) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d? This should be refactored to be a useful name. This can wait for another PR, though, since it was not introduced by this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed - this should be directive.

@codecov
Copy link

codecov bot commented Aug 25, 2020

Codecov Report

Merging #1599 into master will decrease coverage by 0.01%.
The diff coverage is 75.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1599      +/-   ##
==========================================
- Coverage   70.36%   70.34%   -0.02%     
==========================================
  Files         219      220       +1     
  Lines       12163    12176      +13     
==========================================
+ Hits         8558     8565       +7     
- Misses       3605     3611       +6     
Flag Coverage Δ
#full ?
#unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/ast2ram/AstToRamTranslator.h 100.00% <ø> (ø)
src/interpreter/InterpreterNode.h 100.00% <ø> (ø)
src/ram/Visitor.h 17.88% <0.00%> (-0.25%) ⬇️
src/ast/Program.h 16.84% <28.57%> (ø)
src/ast/Directive.h 100.00% <100.00%> (ø)
src/ast/transform/IOAttributes.h 97.74% <100.00%> (+0.06%) ⬆️
src/ast/transform/IODefaults.h 95.00% <100.00%> (+0.12%) ⬆️
src/interpreter/InterpreterGenerator.h 92.30% <100.00%> (+0.07%) ⬆️
src/ram/RelationSize.h 100.00% <100.00%> (ø)
src/include/souffle/datastructure/UnionFind.h 95.74% <0.00%> (-2.13%) ⬇️
... and 4 more

@b-scholz
Copy link
Member Author

I have renamed IO Directives to directives for most of the ast. In future, we can use the directive mechanism for other aspects including data-structure representations etc.

@mmcgr mmcgr merged commit e73ff14 into souffle-lang:master Aug 28, 2020
@b-scholz b-scholz deleted the limitsize branch December 1, 2020 10:22
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.

None yet

2 participants