Added XNOR Function to SymPy

[rishav-sj]

Added the "XNor" Function to sympy/logic/boolalg.py since it was missing

[goodok]

There is a test failure.

See Test1 in
http://reviews.sympy.org/pullrequest/1154

Before pull request please run those commands to be sure that all tests passed:

> ./bin/test
> ./bin/doctest

as it is described in Running tests, Development workflow

[scolobb]

Add a full-stop at the end of the sentence.

[toolforger]

As discussed on the list,

1. the definition of XNOR for more than two arguments should be A XNOR B XNOR C, not NOT(A NOR B NOR C) as defined in the code;
2. XNOR is the same as Equivalent, so the function does not add to SymPy;
3. hence the pull request should be retracted without pulling into SymPy.

[tesseralis]

@toolforger On 2): XNOR and Equivalent are only the same when the number of arguments is 2. We could, say, write it so that it returns true when an even number of arguments is true (in comparison to XOR, which returns true when an odd number or arguments is true), but I don't think this is standard.

[toolforger]

@tesserahub: Nope, they're the same.
Your definition of XNOR(A,B,C) would be NOT(A XOR B XOR C).
The standard definition, however, should be A XNOR B XNOR C, in which case it is indeed the same as Equivalent.

[tesseralis]

We define NAND(A, B, C) as NOT(A AND B AND C), not A NAND B NAND C, so I don't see why we can't define XNOR likewise.

Equivalent(A, B, C) is true only if all of A, B, C are true or all of A, B, C are false. Equivalent(True, False, False) returns False. However, (A XNOR B) XNOR C returns true if A = True, B = False, C = False. Not the same.

[toolforger]

Hm. I see. Not just for Nand but also for other operators.
I'd consider that a bug though. The standard way to extend an operator to multiple operands is
(A op B) op C
not
preop ((A binop B) binop C)

[tesseralis]

You shouldn't consider NAND(A, B, C) to be (A NAND B) NAND C, just as we don't think of EQUIV(A, B, C) as (A EQUIV B) EQUIV C. Things like Logic gates consider NAND(a, b, c...) as NOT(AND(a, b, c, ...)), so I don't think it's a bug. Think of them as n-ary operators rather than a reduction of n-1 binary operators. I think this request should be reopened...

[scolobb]

I tried to throw a look at how the corresponding multiple input logic gates work:

The XNOR gate will emit 1 only when there is not exactly one 1 input

In my reading, this means that XNOR(A1,...,An) is 1 iff either all Ai = 0 or more than one input is 1.

This is certainly different from equivalence and (A XNOR B) XNOR C, but it's still debatable whether this logic is what we should abide by.

I agree with Joachim on the matter that binary operations are usually extended to more than two (finitely many) arguments by inductively applying the operation to pairs: (...((A1 op A2) op A3) ... op An). However, any operation which becomes op in the case of two arguments is formally a valid extension of op.

[toolforger]

That link on multiple input logic gates was quite instructive: "Many authorities contend that the shaped XOR gate's behavior should correspond to the odd parity gate, but there is not agreement on this point."
No agreement indeed - I can see situations where I'd expect XOR to be parity, and situations where I'd expect the exactly-one-of behaviour.

If no agreement can be reached, the case should be undefined, i.e. XOR, XNOR, NAND etc. should be defined to have exactly two parameters.
The ability to write Nor(A, B, C) instead of Not(Or(A, B, C)) is really not worth having expression types that intuitions do not agree about.

To cover the inductive case, we could have a class that accepts an operator and an unlimited list of parameters. That way, we don't have to write an inductive variant for every operator.

[scolobb]

To cover the inductive case, we could have a class that accepts an operator and an unlimited list of parameters. That way, we don't have to write an inductive variant for every operator.

That's a great idea. I think this might be a good thing to do for a patch requirement ;-)

[toolforger]

The class should probably be named Reduce, in analogy to functools.reduce.
It's actually not a new idea. http://en.wikipedia.org/wiki/Fold_%28higher-order_function%29 has all the info you'd ever want to know, and then some.

[rishav-sj]

I'll reopen this by adjusting the design and adding the full stops where appropriate!

[rishav-sj]

@tesserahub I agree with you. If NAND and NOR are defined in this manner so should XNOR. @scolobb I suppose your definition is in harmony with mine. Therefore, I've adjusted the design flaws and reopened the pull request.
Also, @ALL this is for my patch submission requirement as a GSoC student so it would help if we reach a consensus soon. Thanks!

[toolforger]

None of these changes address the basic issue: that it's unclear what the semantics of a multi-parameter XNOR should be.
So this pull request is still -1 from my side.

This does not mean that your patch requirement is not fulfilled; I think @asmeurer has to decide that.
Aspects to consider:
You could have asked whether the patch would be useful before coding. Not a serious problem, that's the kind of mistake that doesn't get repeated.
You haven't spend much thought on the ramifications of the XNOR operations. You lost some bonus points by that, but it does not disqualify you.
I'm a bit unhappy about the scope of the patch. It's essentially a copy&paste job, so it does not demonstrate your Python coding skills.
On the plus side, you know enough to make Python code run, and you know how to work with git and github. So we know that you fulfil the minimum skill requirements, even if we cannot assess how far your skills go beyond that minimum.
I don't know whether this qualifies as fulfilling the patch requirement, it's a bit of a grey area; Aaron should be notified of this discussion since I mentioned him, so he should respond soon.

[rishav-sj]

@toolforger Your inputs are valuable. My project proposal has more to do with creating an Android app so my knowledge of the Python code should, hopefully, be considered less important. I'll wait to see what @asmeurer has to say!

[rishav-sj]

@toolforger @scolobb
Please check out this pull request:
#1207
Is this what you had in mind?

[Krastanov]

SymPy Bot Summary: ✳️ All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYtakUDA

Interpreter: /usr/bin/python2.6 (2.6.7-final-0)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 18afbcc
branch hash: 4f2ff2f

Automatic review by SymPy Bot.

[certik]

This PR as it is cannot be merged, so I am closing this for now. Feel free to reopen it if more work is done.