-
Notifications
You must be signed in to change notification settings - Fork 41
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
Requirement redundancies: Approximation of explanation via invariants #681
Draft
Paswalt
wants to merge
11
commits into
dev
Choose a base branch
from
wip/pw/redundancyExplanation
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+111
−7
Draft
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
992aa12
Added new redundancy result
Paswalt 73ee0dc
Added loop invariant result handling in VerificationResultTransformer…
Paswalt 78b5428
Added location invariants to extract redundancy set approximation
Paswalt a8677c2
Small bugfix
Paswalt 4300c43
Made regex slightly more verbose
Paswalt 3765a8b
Changed function name isLoopLocation to isLoopInvariant inside Invari…
Paswalt e391017
Changed HashSet to LinkedHashSet
Paswalt b4ff0a6
Create InvariantResults for all pre-error locations
Paswalt c80803b
Removed unecessary commented code
Paswalt 8aa2980
Changed predecessors of all error locations to predecessors of the fi…
Paswalt 7a64dc4
Made pattern static
Paswalt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
48 changes: 48 additions & 0 deletions
48
.../src/de/uni_freiburg/informatik/ultimate/pea2boogie/results/ReqCheckRedundancyResult.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
package de.uni_freiburg.informatik.ultimate.pea2boogie.results; | ||
|
||
import java.util.HashSet; | ||
import java.util.Set; | ||
import java.util.regex.Pattern; | ||
import java.util.regex.Matcher; | ||
|
||
import de.uni_freiburg.informatik.ultimate.core.model.models.IElement; | ||
|
||
public class ReqCheckRedundancyResult<LOC extends IElement> extends ReqCheckFailResult<LOC> { | ||
|
||
private String mReqName; | ||
private String mRedundancySet; | ||
|
||
public ReqCheckRedundancyResult(LOC element, String plugin, String reqName, String redundancySet) { | ||
super(element, plugin); | ||
mReqName = reqName; | ||
mRedundancySet = redundancySet; | ||
} | ||
|
||
@Override | ||
public String getLongDescription() { | ||
return "Extracted redundancy set for " + mReqName + ": " + mRedundancySet; | ||
} | ||
|
||
/* | ||
* Function that parses loop invariants represented by strings. In the terms of the regex, | ||
* we extract every name of the automata appearing inside of the loop invariant. | ||
* TODO: One issue that persist is if a user names the observable of a requirement | ||
* such that it matches the regex (for example: "input FakeReq_ct0_total_pc IS bool"). | ||
* For now the method works under the assumption that names created during the formalization process are | ||
* disjoint of the variable names used in the program that simulates the intersection of automata | ||
*/ | ||
public static Set<String> extractRedundancySet(String invariant) { | ||
String regex = "[a-zA-Z_]+[a-zA-Z0-9_]*_ct(0|[1-9][0-9]*)[a-zA-Z0-9_]*_total"; | ||
Pattern pattern = Pattern.compile(regex); | ||
Matcher matcher = pattern.matcher(invariant); | ||
|
||
Set<String> redundancySet = new HashSet<>(); | ||
Paswalt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
while (matcher.find()) { | ||
redundancySet.add(matcher.group().split("_ct")[0]); | ||
} | ||
|
||
return redundancySet; | ||
} | ||
|
||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The easiest and cleanest solution to me seems to be to write a small method (here, or perhaps even in
IcfgUtils
) which collects all predecessors of error locations in aSet<IcfgLocation>
, and then just add them tolocsForLoopLocations
(just likegetPotentialCycleProgramPoints
above).At that point, we could then rename the set to sth like
locsForInvariants
;-)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, this seems like the best solution. In our setting, we only have a single unique predecessor for all error locations whose invariant we utilize. That means for each requirement check, we do something with the resulting invariant of said location. There's not really any combining involved as it is obscuring the results. However, I don't think that changing the code to handle all predecessors of error locations in general would change anything for our setting, so a cleaner implementation like that can easily be done while still being in agreement to our use case, at least if I understand correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have implemented a function in IcfgUtils that returns the predecessors of all error locations in b4ff0a6
After looking more into the code, the issue is more complicated than I initially thought, however. I try to explain in a short manner:
In our redundancy analysis, assuming we look at N requirements, we have N error locations. While conceptually it is true that there is only one predecessor, in truth there is a predecessor for each of the N error locations. However, all of the predecessors will always have the same location invariant in our setting (we run one CEGAR per error location + the structure of our problem leads to this observation). Hence, we only need a single invariant result from any of these predecessor locations.
The way it is coded now, we create an invariant result for all predecessors. What happens in Pea2Boogie is the following: we have a method that takes trace abstraction results and transforms them into our desired results. (Now, if we check the redundancy of let's say a requirement called r1 with a total of N = 1000 requirements, and it turns out that r1 is indeed redundant, trace abstraction will throw 1000 location invariant results for the predecessors of the error locations into this method. However, all of them will have the same location invariant, which we only want to receive once to output it.
ultimate/trunk/source/PEAtoBoogie/src/de/uni_freiburg/informatik/ultimate/pea2boogie/VerificationResultTransformer.java
Line 130 in c80803b
ultimate/trunk/source/PEAtoBoogie/src/de/uni_freiburg/informatik/ultimate/pea2boogie/PEAtoBoogieObserver.java
Line 99 in c80803b
To circumvent the issue, I added a temporary set into the VerificationResultTransformer until I can think of a better way to handle this. This way the same location invariant is not output N times but only once per requirement that is redundant. Though, the issue remains as all of the 1000 invariant results still enter the method per redundant requirement, and each time 999 are then rejected which is just a waste and not acceptable if the possibility exists to just create one for our use case. In order to think of a better solution I have the following questions:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FloydHoareUtils::createInvariantResults
you have access to the checked specification asspec = annotation.getSpecification()
spec::isFinalState
. Have a look atFloydHoareUtils::getCheckedSpecifications
for an example.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that worked, see 8aa2980
I left the new function in IcfgUtils still there in case it is useful in the future. But if the argument that it's non-usage at the moment makes it superfluous, I can also still remove it. This current version now perfectly matches what I wanted.
In summary, I think in the future a setting for the predecessors of the locations obtained via the spec through isFinalState would be helpful for our purpose. Thank you both so much for your help!