Fixed PropKB Ask. Added function to check validity. #2830

sd-biswas · 2014-01-25T19:50:12Z

Added function to check for validity of propositional sentences
Fixed bug where all queries on an empty KB would return False
Fixed PropKB Ask function to correctly give the logical entailment
Added Tests for the above

srjoglekar246 · 2014-01-25T20:26:10Z

Is there a reason I am unable to give line by line comments? @asmeurer

Anyways, some technical stuff-

The format you have used for the Examples section is not quite correct. Do have a look at the way the heading is given in other docs and follow it strictly.
Avoid writing long statements like the second return statement for the 'valid' function. Its better, and more in agreement with the rest of the codebase, to do it the indentation way.
Follow PEP8 conventions for the tests and other places too. (Like A >> B instead of A>>B)

asmeurer · 2014-01-25T20:27:09Z

I can't do it either. I think GitHub's javascript must be broken right now.

sd-biswas · 2014-01-25T20:46:32Z

I looked at satisfiable function docs for reference. Apparently even that is not in tune with the docs elsewhere. Will fix both.
Will convert all the conditional expressions to standard if, else.
My bad, will fix it immediately

srjoglekar246 · 2014-01-26T09:17:17Z

sympy/logic/inference.py

        s = set()
        for q in query_conjuncts:
            s = s.union(q.atoms(C.Symbol))
-        return bool(dpll(query_conjuncts, list(s), {}))
+        return True if dpll(query_conjuncts, list(s), {}) is False else False


You may want to add an inline comment explaining why this is so. We don't want any confusion about the algorithms and logic behind our code, as is the case for most of the logic module as of now.

srjoglekar246 · 2014-01-26T09:22:34Z

You may want to bring your function into the sympy.logic namespace by entering it into the init file in sympy.logic folder.

Also, I think one of the tests in test_PropKB is failing. Please look into that and fix the issue.

sd-biswas · 2014-01-26T19:40:26Z

Fixed the coding convention issues
Added valid function to the logic init file
Fixed the failing test case (left over from old test cases) and deleted redundant test cases.

srjoglekar246 · 2014-01-27T08:03:41Z

This seems to be fine now. I am assuming you have tested all the test cases by hand and confirmed their validity.
I am +1.
@asmeurer unless you have something to add, I guess you could merge this.

skirpichev · 2014-01-27T09:03:18Z

sympy/logic/inference.py

+
+
+
+


I think, 2 newlines enough...

+1. @SDB1323 , have one newline between methods of a class, and two newlines between module-level functions/classes.

sd-biswas · 2014-01-30T18:03:08Z

Travis seems to be giving the following error:
"No output has been received in the last 10 minutes, this potentially indicates a stalled build or something wrong with the build itself. The build has been terminated"

Could someone please indicate whether some action is required from my side, or it is normal to get this message and the PR will be merged?

srjoglekar246 · 2014-01-30T18:05:08Z

Ping @skirpichev

moorepants · 2014-01-30T19:16:45Z

Travis is timing out a lot these days. I'm not sure what the major issue is.

skirpichev · 2014-01-30T19:41:48Z

timeouts in test_gruntz.py/test_wester.py should be rare after merging #2851

asmeurer · 2014-01-31T01:34:48Z

I haven't reviewed it, but if you are OK with the changes, then please merge.

sd-biswas · 2014-02-13T16:54:44Z

Could this PR be merged? I need the valid function for a future PR. Travis passes all but one test, which times out.
Ping @skirpichev

sd-biswas · 2014-02-13T17:47:56Z

I am really sorry. But all my ways to make Travis rebuild failed. And since most of the tests had timed out there was no way to know whether the build was correct or not. Hence the desperate attempt. I apologize.

skirpichev · 2014-02-13T19:29:02Z

@SDB1323, can you rebase this pr to get rid of meaningless commits?
e.g. test commit, dummy commit, formatting fixes...

asmeurer · 2014-05-23T04:14:47Z

sympy/logic/inference.py

@@ -92,6 +93,30 @@ def satisfiable(expr, algorithm="dpll2"):
    raise NotImplementedError


+def valid(expr):
+    """
+    Check validity of a propositional sentence.


Does valid mean tautological? If so, I think that "tautological" is a better word to use here.

I guess "valid" is more used in logic, though https://en.wikipedia.org/wiki/Valid. It should be made clear in the docstring what this means, though, in case someone isn't familiar with the logic terminology.

asmeurer · 2014-05-23T04:19:01Z

I think Travis failed because of some old test failure from two months ago. Once you push commits addressing my comments, that should go away.

asmeurer · 2014-05-23T04:21:04Z

sympy/logic/inference.py

+    if isinstance(expr, bool):
+        return expr
+
+    if satisfiable(Not(expr)) is False:


return not satisfiable(Not(expr))

asmeurer · 2014-05-23T04:22:47Z

My main concern here is the changed test cases for kb.ask. What does that function do? Can you explain why the previous test cases were wrong?

asmeurer · 2014-06-18T19:54:46Z

This needs to be merged with master.

sd-biswas · 2014-06-27T03:49:53Z

@asmeurer I have fixed the merge conflicts as well as added a couple of pending things. Please review.

asmeurer · 2014-06-27T15:55:43Z

sympy/logic/inference.py

+    .. [1] en.wikipedia.org/wiki/Validity
+
+    """
+    if isinstance(expr, bool):


This shouldn't be needed.

sd-biswas · 2014-07-01T08:21:16Z

@asmeurer : This seems to have passed the Travis build. Please review and merge if no further changes are required. The last commit should fix the {false: true} mapping problem, along with the python 2.6 compatibility fix.

asmeurer · 2014-07-01T15:17:15Z

Did you add a test for the {false: true} issue?

Fixed PropKB Ask. Added function to check validity.

srjoglekar246 reviewed Jan 26, 2014
View reviewed changes

skirpichev reviewed Jan 27, 2014
View reviewed changes

sd-biswas mentioned this pull request Feb 11, 2014

Added support for Equivalent with multiple arguments in functions #2869

Merged

asmeurer reviewed May 23, 2014
View reviewed changes

Fixed PropKB. Added valid and entails

e77d196

debugger22 added GSoC labels Jun 27, 2014

asmeurer reviewed Jun 27, 2014
View reviewed changes

sympy/logic/inference.py

.. [1] en.wikipedia.org/wiki/Validity

"""

if isinstance(expr, bool):

Copy link

Member

asmeurer Jun 27, 2014

Choose a reason for hiding this comment

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

This shouldn't be needed.

Fix python 2.6 compatibility issues

f8b4bef

Added tests for bool input to satisfiable

e8b3f52

asmeurer added a commit that referenced this pull request Jul 6, 2014

Merge pull request #2830 from SDB1323/logic

6aa63b5

Fixed PropKB Ask. Added function to check validity.

asmeurer merged commit 6aa63b5 into sympy:master Jul 6, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed PropKB Ask. Added function to check validity. #2830

Fixed PropKB Ask. Added function to check validity. #2830

sd-biswas commented Jan 25, 2014

srjoglekar246 commented Jan 25, 2014

asmeurer commented Jan 25, 2014

sd-biswas commented Jan 25, 2014

srjoglekar246 Jan 26, 2014

srjoglekar246 commented Jan 26, 2014

sd-biswas commented Jan 26, 2014

srjoglekar246 commented Jan 27, 2014

skirpichev Jan 27, 2014

srjoglekar246 Jan 27, 2014

sd-biswas commented Jan 30, 2014

srjoglekar246 commented Jan 30, 2014

moorepants commented Jan 30, 2014

skirpichev commented Jan 30, 2014

asmeurer commented Jan 31, 2014

sd-biswas commented Feb 13, 2014

sd-biswas commented Feb 13, 2014

skirpichev commented Feb 13, 2014

asmeurer May 23, 2014

asmeurer May 23, 2014

asmeurer commented May 23, 2014

asmeurer May 23, 2014

asmeurer commented May 23, 2014

asmeurer commented Jun 18, 2014

sd-biswas commented Jun 27, 2014

asmeurer Jun 27, 2014

sd-biswas commented Jul 1, 2014

asmeurer commented Jul 1, 2014

Fixed PropKB Ask. Added function to check validity. #2830

Fixed PropKB Ask. Added function to check validity. #2830

Conversation

sd-biswas commented Jan 25, 2014

srjoglekar246 commented Jan 25, 2014

asmeurer commented Jan 25, 2014

sd-biswas commented Jan 25, 2014

srjoglekar246 Jan 26, 2014

Choose a reason for hiding this comment

srjoglekar246 commented Jan 26, 2014

sd-biswas commented Jan 26, 2014

srjoglekar246 commented Jan 27, 2014

skirpichev Jan 27, 2014

Choose a reason for hiding this comment

srjoglekar246 Jan 27, 2014

Choose a reason for hiding this comment

sd-biswas commented Jan 30, 2014

srjoglekar246 commented Jan 30, 2014

moorepants commented Jan 30, 2014

skirpichev commented Jan 30, 2014

asmeurer commented Jan 31, 2014

sd-biswas commented Feb 13, 2014

sd-biswas commented Feb 13, 2014

skirpichev commented Feb 13, 2014

asmeurer May 23, 2014

Choose a reason for hiding this comment

asmeurer May 23, 2014

Choose a reason for hiding this comment

asmeurer commented May 23, 2014

asmeurer May 23, 2014

Choose a reason for hiding this comment

asmeurer commented May 23, 2014

asmeurer commented Jun 18, 2014

sd-biswas commented Jun 27, 2014

asmeurer Jun 27, 2014

Choose a reason for hiding this comment

sd-biswas commented Jul 1, 2014

asmeurer commented Jul 1, 2014