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

Incorporating a function for Tautological Implication in the Logic Module #28052

Closed
Medha210699 mannequin opened this issue Jun 25, 2019 · 26 comments
Closed

Incorporating a function for Tautological Implication in the Logic Module #28052

Medha210699 mannequin opened this issue Jun 25, 2019 · 26 comments

Comments

@Medha210699
Copy link
Mannequin

Medha210699 mannequin commented Jun 25, 2019

This ticket defines a method is_consequence for the class BooleanFormula. This method provides the same functionality as the function valid_consequence (which is now deprecated): the method returns True if self is a tautological consequence of the given formula(s).


sage: f = propcalc.formula("a")
sage: g = propcalc.formula("a|b")
sage: f.is_consequence(g)
False
sage: g.is_consequence(f)
True

In addition, the function call is_consequence(conclusion,hypotheses) is defined to be a synonym for conclusion.is_consequence(hypotheses) and is therefore a drop-in replacement for the deprecated function valid_consequence.

Component: symbolics

Keywords: logic, symbolics, propositional formula

Author: Medha Sharma, Dave Morris

Branch/Commit: bbec2e6

Reviewer: Dave Morris, Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/28052

@Medha210699 Medha210699 mannequin added this to the sage-8.9 milestone Jun 25, 2019
@Medha210699 Medha210699 mannequin added c: symbolics labels Jun 25, 2019
@Medha210699
Copy link
Mannequin Author

Medha210699 mannequin commented Jun 25, 2019

Author: Medha Sharma

@Medha210699 Medha210699 mannequin added the s: needs review label Jun 25, 2019
@Medha210699

This comment has been minimized.

@videlec
Copy link
Contributor

videlec commented Jun 25, 2019

comment:3

A ticket should be set in needs review when it provides source code ready to be integrated into Sage.

@Medha210699 Medha210699 mannequin self-assigned this Jun 27, 2019
@Medha210699
Copy link
Mannequin Author

Medha210699 mannequin commented Jun 27, 2019

@embray
Copy link
Contributor

embray commented Dec 30, 2019

comment:6

Ticket retargeted after milestone closed

@embray embray modified the milestones: sage-8.9, sage-9.1 Dec 30, 2019
@DaveWitteMorris
Copy link
Member

Reviewer: Dave Morris

@DaveWitteMorris
Copy link
Member

comment:7

I recommend closing this ticket as "invalid/wontfix", because the functionality is already available by using sage.logic.propcalc.valid_consequence.

sage: from sage.logic.propcalc import valid_consequence
sage: f = propcalc.formula("a")
sage: g = propcalc.formula("a|b")
sage: valid_consequence(g,f) # is g a tautological consequence of f ?
True
sage: valid_consequence(f,g) # is f a tautological consequence of g ?
False

@tscrim
Copy link
Collaborator

tscrim commented Mar 13, 2020

comment:8

Perhaps it would be good to add a method valid_consequence that redirects to this function to make it more discoverable?

@DaveWitteMorris
Copy link
Member

comment:9

Thanks, that seems like a good idea. I'll do it soon, but probably not today. Probably it makes the most sense to move the definition into a method, and then (for backward compatibility) define a stand-alone function that calls the method. Perhaps the stand-alone should be deprecated?

@tscrim
Copy link
Collaborator

tscrim commented Mar 13, 2020

comment:10

I don't use this, so I don't have an option. I think it is okay to have a method redirect to a function (or vice versa; they are equivalent in my mind up to putting the definition in the method to make it easier to see via ? on the command-line). There might be some reason for having the actual computation done in a function (which could be in a separate file). Actually, the easiest thing to do would be to simply make the function into a (bounded) method of the appropriate class. I think you just need:

def foo(x, y):
    return -y
class Bar:
    foo = foo
sage: b = Bar()
sage: b.foo(2)
-2

This would mean everything in the doc, etc. is properly linked without redundancy. This would be my recommendation.

@DaveWitteMorris
Copy link
Member

@DaveWitteMorris
Copy link
Member

Commit: e9d0529

@DaveWitteMorris
Copy link
Member

comment:12

Thanks for telling me about foo = foo. I was going to do something more complicated.

  1. I defined a method of the class BooleanFormula by moving the code for valid_consequence from propcalc.py into boolformula.py.
  2. I named the method is_consequence (and also defined a standalone function of this same name), to be consistent with the methods is_satisfiable, is_contradiction, etc., but retained valid_consequence as a deprecated function for backwards compatibility (in the file propcalc.py).
  3. I refactored the code for the is_consequence method, added coverage of the case where there are no hypotheses, and added more doctests.
  4. I made some other minor tweaks to propcalc.py and boolformula.py, such as changing "http" to "https".
  5. I revised the description of this ticket.
    Should some of this be on a separate ticket?

New commits:

e9d0529move function to method of BooleanFormula

@DaveWitteMorris

This comment has been minimized.

@DaveWitteMorris
Copy link
Member

Changed author from Medha Sharma to Medha Sharma, Dave Morris

@DaveWitteMorris
Copy link
Member

comment:14

Merge conflict.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 19, 2020

Changed commit from e9d0529 to 6c0e20a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 19, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

7dc3dcdfix conflict with #28053
6c0e20amerge with 9.1b8

@DaveWitteMorris
Copy link
Member

comment:16

Fixed the merge conflict with #28053 and merged 9.1b8. I hope the patchbots will be happy now.

@tscrim
Copy link
Collaborator

tscrim commented Mar 19, 2020

Changed reviewer from Dave Morris to Dave Morris, Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Mar 19, 2020

comment:17

Some little comments:

-if not(hypotheses):
+if not hypotheses:

Remove self as an INPUT:.

Beyond that, LGTM.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 19, 2020

Changed commit from 6c0e20a to bbec2e6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 19, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

bbec2e6minor corrections pointed out by reviewer

@DaveWitteMorris
Copy link
Member

comment:19

Thanks for the corrections.

@tscrim
Copy link
Collaborator

tscrim commented Mar 19, 2020

comment:20

Thank you. LGTM.

@vbraun
Copy link
Member

vbraun commented Mar 25, 2020

Changed branch from public/28052 to bbec2e6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants