Added a simplify pattern for XOR that reduces A xor 0 to A. - #498
Added a simplify pattern for XOR that reduces A xor 0 to A.#498jclapis wants to merge 2 commits into
Conversation
|
In fact, would get reduced to gets reduced to |
|
That is true for the bitwise operation, certainly. Our group is using AngouriMath for some quantum computing research, so we are using XOR to compare two quantum registers which are usually represented as variables and numbers instead of the bitwise approach. In either case, I just tested it with the following: [Fact] public void Xor3() => AssertSimplify(new Entity.Xorf(false, x), x);
[Fact] public void Xor4() => AssertSimplify(new Entity.Xorf(x, false), x);The pattern above still passes these tests, but fails the |
Codecov Report
@@ Coverage Diff @@
## master #498 +/- ##
=============================
=============================
Continue to review full report at Codecov.
|
|
|
||
| // a xor true = not a | ||
| Xorf(var any1, var any2) when any2 == true => new Notf(any1), | ||
| Xorf(var any2, var any1) when any2 == true => new Notf(any1), |
There was a problem hiding this comment.
You can write it as Xorf(var any1, Boolean(true)) => any1.Not() thanks to pattern matching. But don't those rules in fact already exist? Let me check
Sounds amazing. |
|
So here's the thing. The most basic operations (like exclusive or against a constant, or multiplication by 0, or etc.) are covered by inner simplification than by patterns, and it seems to be covered already. Although now I think that having it in patterns is not a bad idea too 🤔 . |
| Xorf(var any1, Entity.Boolean any2) when any2 == false => any1, | ||
| Xorf(var any2, var any1) when any2 == false => any1, | ||
| Xorf(var any1, Integer any2) when any2 == 0 => any1, | ||
| Xorf(Integer any2, var any1) when any2 == 0 => any1, |
There was a problem hiding this comment.
There's no logic backing xor on numbers, so this pattern is not particularly useful. Or you have some other, broader idea regarding xor? Please, share!
There was a problem hiding this comment.
I'm not sure what you mean about backing logic; are you referring to the solver system? If so, we're just using the symbolic algebra representation and simplification systems so this isn't something we had considered. In quantum notation, you see things like this quite frequently:
That is, some variable XOR'd against some other variable. In some cases one of those variables is simply the integer 0, and that can be simplified to just using the other value. That's the use case this was intended to support.
There was a problem hiding this comment.
Right. What I am saying is that we don't support boolean operators on integers. So if we want to add it, it should be added for all boolean operators and for all cases, not only for a few cases for xor. That's what I mean 😅 . But before doing so, we need to think how it will be designed.
There was a problem hiding this comment.
By the way, maybe you want to join our discord server (see the repo's readme's green badge)? It's a chat, so we can discuss it more conveniently there (and having others potentially involved)
There was a problem hiding this comment.
Sure, I'll hop in tomorrow if you believe there's more to this discussion that what you've mentioned here.
Probably, I hadn't tested them before because we don't use
This is our sample code: It produces the following: We would like it to produce |
|
Let's keep this PR open for now, because I'm not sure whether we need it for integers (it doesn't work on integers currently ; to make it work, you would need to implement all logic for boolean operators). In your case, you can do |
|
If this were to be added, the xor operator would probably be made to work like a bitwise operator, and other logical operators like |
|
Validated against today's Four of the six rules are already true without the patch. On current
The boolean cases come out of inner simplification, exactly as described above, so those four pattern lines never fire and the four tests asserting them pass on And those two are the part worth thinking about. They make So the shape of a mergeable change here is bitwise integer logic as a whole, not this one identity. If that is wanted, I am happy to write it; it is a contained piece of work. @jclapis — for the original use, if the two registers are compared as booleans rather than integers, the four boolean reductions you want are already there today with no patch at all. Worth checking whether that covers you five years on. No action taken on this PR either way; this is validation, not a verdict. |
|
needs an analysis on how mathematicians vs physicians approach this operator. |
|
@Happypig375 — the analysis you asked for. The split is not mathematicians vs physicistsBoth communities use ⊕ in both senses, and the real dividing line is what the operands are. On truth values, ⊕ is exclusive disjunction. Propositional logic and Boolean algebra, in both fields. On numbers, ⊕ is bitwise XOR — addition in GF(2)ⁿ — and mathematicians use it that way constantly:
And physicists — specifically the field this PR comes from — use exactly the same numeric reading. The CNOT and the standard oracle are written A third meaning, worth naming because it is arguably the most common in pure mathematics: ⊕ is the direct sum — So there is no community that reads ⊕ on two integers as a boolean operation. If anything, the boolean-only reading is the outlier. What the library does todayMeasured on current master — and it is consistent, which is worth saying:
Boolean operators are boolean; given integers they decline, uniformly. No bitwise route exists. What the other systems do — two operators, not one
Nobody overloads one operator across both. And note that SymPy's Where that leaves this PRThe rule it adds, RecommendationAdd That answers your objection directly: with a split, @jclapis — for the original problem: if the two registers are compared as booleans rather than integers, all four reductions you wanted already work on master today with no patch. The bitwise route is what is missing. I am happy to implement the bitwise family. It is contained: a node kind, evaluation over integers, the parser names, both compilers, and the identities ( |


This is a small PR that adds a rule regarding XOR to the common simplifier patterns:
X ⊕ 0 = XOur group encounters this kind of pattern occasionally, so we would like to see this added to the canonical
Simplify()method. All of the unit tests pass with this new rule in place, and we have added some unit tests to cover this new behavior.