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

Add additional simple support nogoods #226

Closed
wants to merge 6 commits into from
Closed

Conversation

rtaupe
Copy link
Collaborator

@rtaupe rtaupe commented Feb 21, 2020

If there are multiple ground rules in the input program (i.e., before grounding!), each with a unique ground head, we can generate completion nogoods for each of them. So far this did not happen if the unique ground heads were of the same predicate.

@codecov
Copy link

codecov bot commented Feb 24, 2020

Codecov Report

Merging #226 (68c62d2) into master (1fbbbf0) will decrease coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #226      +/-   ##
============================================
- Coverage     77.46%   77.36%   -0.10%     
+ Complexity     2312     2311       -1     
============================================
  Files           178      178              
  Lines          7782     7792      +10     
  Branches       1352     1355       +3     
============================================
  Hits           6028     6028              
- Misses         1334     1337       +3     
- Partials        420      427       +7     
Impacted Files Coverage Δ Complexity Δ
...t/ac/tuwien/kr/alpha/grounder/NoGoodGenerator.java 98.36% <ø> (ø) 23.00 <0.00> (ø)
.../at/ac/tuwien/kr/alpha/grounder/NaiveGrounder.java 86.66% <100.00%> (+0.54%) 75.00 <7.00> (+6.00)
...ien/kr/alpha/solver/heuristics/BerkMinLiteral.java 88.23% <0.00%> (-11.77%) 8.00% <0.00%> (-1.00%)
...a/solver/learning/GroundConflictNoGoodLearner.java 76.79% <0.00%> (-2.21%) 42.00% <0.00%> (-3.00%)
...uwien/kr/alpha/solver/NoGoodStoreAlphaRoaming.java 80.24% <0.00%> (-0.81%) 120.00% <0.00%> (-3.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fbbbf0...68c62d2. Read the comment docs.

@rtaupe rtaupe changed the title Add additional simple support nogoods WIP: Add additional simple support nogoods Feb 24, 2020
@rtaupe
Copy link
Collaborator Author

rtaupe commented Mar 17, 2020

Build fails because of #239, but I do not think the cause for this issue comes from this pull request.

@rtaupe rtaupe requested a review from AntoniusW March 17, 2020 09:02
@rtaupe rtaupe changed the title WIP: Add additional simple support nogoods Add additional simple support nogoods Apr 10, 2020
@rtaupe rtaupe requested review from madmike200590 and removed request for AntoniusW December 4, 2020 15:29
@rtaupe rtaupe requested review from AntoniusW and removed request for madmike200590 December 21, 2020 13:03
@AntoniusW
Copy link
Collaborator

Thank you for the reminder. This PR adds certain completion nogoods, which is a good step in the right direction. There is, however, also a more principled approach in the direction of adding completion noogods currently under development (see PR #283 ). What is added here, also gets generated in #283 and more. I am currently working on the completion branch/PR (it needs a complete restructuring itself to cover more cases), with the goal of getting it ready in the next month. It is quite likely that this contribution here will be overwritten/redone/removed with #283. Do you still want it merged at this time?

@rtaupe
Copy link
Collaborator Author

rtaupe commented Dec 22, 2020

Yes please, because having ready features merged to master reduces maintenance efforts when we need them in other branches.

Thanks for your work on #283 though!

@rtaupe
Copy link
Collaborator Author

rtaupe commented Dec 26, 2020

I just found out that this pull request is buggy, because it does not check for multiple ground rules whose heads are of the same predicate if the heads are actually different.

For example, consider the following program:

h :- not a.
h :- not b.

With this pull request, completion nogoods for both rules are created individually, which is wrong.

Therefore, @AntoniusW I´m retracting this pull request and hope to be able to use your feature soon! I suggest you incorporate a unit test for this very condition in your branch.

@rtaupe rtaupe closed this Dec 26, 2020
rtaupe added a commit that referenced this pull request Jan 8, 2021
(to be superseded by #283)
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