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

Added a simplify pattern for XOR that reduces A xor 0 to A. #498

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jclapis
Copy link

@jclapis jclapis commented Aug 23, 2021

This is a small PR that adds a rule regarding XOR to the common simplifier patterns:

X ⊕ 0 = X

Our 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.

@WhiteBlackGoose
Copy link
Member

In fact, xor is not exactly a numeric operation. It's an operation on booleans, so

a xor false

would get reduced to a, and

a xor true

gets reduced to not a

@jclapis
Copy link
Author

jclapis commented Aug 23, 2021

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 a xor true tests. I will amend this PR with patterns for that in place shortly.

@codecov-commenter
Copy link

Codecov Report

Merging #498 (1b3909d) into master (e45a06c) will not change coverage.
The diff coverage is n/a.

❗ Current head 1b3909d differs from pull request most recent head 9e4b6b8. Consider uploading reports for the commit 9e4b6b8 to get more accurate results
Impacted file tree graph

@@      Coverage Diff      @@
##   master   #498   +/-   ##
=============================
=============================

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 e45a06c...9e4b6b8. Read the comment docs.


// 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),
Copy link
Member

Choose a reason for hiding this comment

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

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

@WhiteBlackGoose
Copy link
Member

Our group is using AngouriMath for some quantum computing research

Sounds amazing.

@WhiteBlackGoose
Copy link
Member

WhiteBlackGoose commented Aug 24, 2021

As far as I can see, those patterns are already there, aren't they?

(this is an F# wrapper of the library, but it's powered by the same kernel)

@WhiteBlackGoose
Copy link
Member

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,
Copy link
Member

Choose a reason for hiding this comment

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

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!

Copy link
Author

Choose a reason for hiding this comment

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

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:

image

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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)

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I'll hop in tomorrow if you believe there's more to this discussion that what you've mentioned here.

@jclapis
Copy link
Author

jclapis commented Aug 24, 2021

As far as I can see, those patterns are already there, aren't they?

Probably, I hadn't tested them before because we don't use Boolean as one of the arguments for Xorf. We use Integer, as in this pattern:

X ⊕ 0 = X

This is our sample code:

Entity.Variable n = "n";
Entity.Xorf xor = new Entity.Xorf(n, 0);
xor.Simplify();
Console.WriteLine(xor);

It produces the following:

n xor 0

We would like it to produce n instead of n xor 0. Clearly it works out of the box with Boolean as you have demonstrated, but it doesn't work for Integer. That's what the original incarnation of this PR aimed to add.

@WhiteBlackGoose
Copy link
Member

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

Entity ReplaceIntsWithBooleans(Entity expr) => expr.Substitute(0, false).Substitute(1, true);
Entity ReplaceBooleansWithInts(Entity expr) => expr.Substitute(false, 0).Substitute(true, 1);

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.

None yet

3 participants