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
add xor,xnor and tests #1206
add xor,xnor and tests #1206
Conversation
symengine/logic.cpp
Outdated
if (eq(*a, *boolTrue)) { | ||
cnttrue++; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function only checks for trues and false. logical_xor(x, y)
would give false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't do git add .
. There's a binary file commited here, symengine/stap7FQc
c04f601
to
4b19a41
Compare
symengine/logic.cpp
Outdated
@@ -352,6 +404,43 @@ RCP<const Boolean> logical_not(const RCP<const Boolean> &s) | |||
} | |||
} | |||
|
|||
RCP<const Boolean> logical_xor(const set_boolean &s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use a set
here, how would you pass x, x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to change this....Same problem is with all other gates!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or
and And
is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
symbols can be used as parameters if set_boolean is replaced by set_basic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From x
I meant the same boolean. for example, y < 0, y < 0
where y
is a symbol.
symengine/logic.cpp
Outdated
for (auto &a : s) { | ||
if (is_a<BooleanAtom>(*a)) { | ||
auto val = down_cast<const BooleanAtom &>(*a).get_val(); | ||
if (val == false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is false
ignored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because Xor(args,false) = Xor(args)
symengine/logic.cpp
Outdated
if (is_a<BooleanAtom>(*a)) { | ||
SYMENGINE_ASSERT(a == boolTrue) | ||
args.erase(a); | ||
return logical_xnor(get_vec_from_set(args)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For performance reasons, these type of recursion should be avoided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.. i would convert it to iterative way..
symengine/logic.cpp
Outdated
for (auto &a : args) { | ||
if (is_a<BooleanAtom>(*a)) { | ||
SYMENGINE_ASSERT(a == boolTrue) | ||
args.erase(a); | ||
return logical_xnor(get_vec_from_set(args)); | ||
cantsimplify = false; | ||
nots++; | ||
} else if (args.find(logical_not(a)) != args.end()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be checked in Line 447 and don't need to do this here.
symengine/logic.cpp
Outdated
for (auto &a : args) { | ||
if (is_a<BooleanAtom>(*a)) { | ||
SYMENGINE_ASSERT(a == boolTrue) | ||
args.erase(a); | ||
return logical_xnor(get_vec_from_set(args)); | ||
cantsimplify = false; | ||
nots++; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can do this in the if
condition in line 429
symengine/logic.cpp
Outdated
} else if (args.size() == 1) { | ||
return *args.begin(); | ||
} else { | ||
while (args.size() > 1 and not cantsimplify) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this run multiple times?
symengine/logic.cpp
Outdated
return *args.begin(); | ||
} else { | ||
while (args.size() > 1 and not cantsimplify) { | ||
cantsimplify = true; | ||
for (auto &a : args) { | ||
if (is_a<BooleanAtom>(*a)) { | ||
SYMENGINE_ASSERT(a == boolTrue) | ||
args.erase(a); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are erasing from the vector while iterating it. Segfaults can happen if you do that.
009b198
to
a431ed8
Compare
ping @isuruf |
symengine/logic.cpp
Outdated
args.erase(aa); | ||
} else { | ||
args.insert(aa); | ||
if (args.find(logical_not(aa)) != args.end()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Store args.find(logical_not(aa))
so that you don't have to call this below as wel.
symengine/logic.cpp
Outdated
if (args.find(aa) != args.end()) { | ||
args.erase(aa); | ||
} else { | ||
args.insert(aa); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Insert this only if the following if command is not true
symengine/logic.cpp
Outdated
args.erase(a); | ||
} else { | ||
args.insert(a); | ||
if (args.find(logical_not(a)) != args.end()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
symengine/logic.cpp
Outdated
if (args.find(a) != args.end()) { | ||
args.erase(a); | ||
} else { | ||
args.insert(a); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
ping @isuruf |
add xor,xnor and tests
No description provided.