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

allow short-circuiting of boolean expressions #11

Merged
merged 1 commit into from
Dec 13, 2022
Merged

allow short-circuiting of boolean expressions #11

merged 1 commit into from
Dec 13, 2022

Conversation

exaexa
Copy link

@exaexa exaexa commented Dec 2, 2022

This is a tiny opportunistic optimization -- current grammar parsing (left-recursive) disables any kind of short-circuiting that could happen during evaluation. Switching to right-recursive allows the value of the left-expression to decide whether or not to evaluate the rest of the expression, possibly saving time.

The exact spot that can do the shortcircuit now is:
https://github.com/sysbio-curie/MaBoSS-env-2.0/blob/master/engine/src/BooleanNetwork.h#L1442
(and the appropriate one for Or).

Impact

before

 ~/work/MaBoSS-env-2.0/engine/pub $ time ./MaBoSS_90n ../examples/Sizek/sizek.bnd -c ../examples/Sizek/sizek.cfg -o res -e sample_count=10000 -e thread_count=1

real	1m38.151s
user	1m33.794s
sys	0m4.348s

perf showed that around 98% of the recursive calls in the AndLogicalExpression::eval were for the "left" branch, spending around 40% total time in the function.

after (~1.7× speedup)

~/work/MaBoSS-env-2.0/engine/src $ time ./MaBoSS_90n ../examples/Sizek/sizek.bnd -c ../examples/Sizek/sizek.cfg -o res -e sample_count=10000 -e thread_count=1

real	0m56.860s
user	0m52.708s
sys	0m4.095s

perf shows around 20% total time spent in AndLogicalExpression::Eval and the left/right evaluations are kinda more balanced (seems veeeery roughly like half and half, which is good).

The speedup is model-dependent, models may behave better or worse depending on how the boolean expressions there are written. But there should never be a slowdown as the original behavior was worst-case.

Considerations

if any of the expressions' evaluations are supposed to have side effects, this likely breaks it. I didn't find such behavior, but better check, right?

Eventually it would be very nice to actually reorder the expressions based on complexity (so that the most probable short-circuiting ones are tried first, saving as much power as possible).

@exaexa
Copy link
Author

exaexa commented Dec 2, 2022

(cc @vincent-noel @ArnauMontagud )

@vincent-noel vincent-noel merged commit e6902b7 into sysbio-curie:master Dec 13, 2022
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.

2 participants