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

draft: Perf: optimize petri net replay #445

Open
wants to merge 11 commits into
base: release
Choose a base branch
from

Conversation

EduardoGoulart1
Copy link
Contributor

This was worked on during the Celonis Impact Day on the 29th of September 2023

This PR suggests a few changes to the Petri net semantics class to optimize the Petri net replay. The changes go from less to more radical, so it's up for discussion about which changes should be accepted. The optimizations is guided by the benchmarks. Given a Petri net (the 5 added), it measures the total runtime to compute the reachable markings of the Petri net. The results can be found below (runtime in seconds):

Model # States Initial Time Decorators Slots Faster Hashing Cache The ID Iterate and Unpack Items Native Copy Marking as Dictionary Compiled Semantics Use Place IDs
M1 144 0.0076 0.0077 0.0072 0.0070 0.0065 0.0061 0.0054 0.0042 0.0019 0.0015
M2 230 0.013 0.013 0.012 0.012 0.011 0.010 0.0090 0.0067 0.0038 0.0028
M7 36740 4.69 4.83 4.56 4.24 3.83 3.48 2.99 2.18 1.30 0.94
ML2 82939 16.63 16.75 15.91 14.87 13.46 12.79 12.06 10.33 2.45 1.83
ML4 1154 0.076 0.076 0.074 0.069 0.063 0.059 0.051 0.036 0.023 0.018

If you ask me, only slots are not worth the trouble, because it will potentially break existing code (if the users have been misusing the class). The change from counter to dict would also require some rework before deploying. In the end, we would end up reimplementing the Counter Python class, but that is alright because the counter class is just very slow. The usage of place IDs instead of places for the markings could be hidden behind some nice abstractions (example: a "CompiledMarking" class)

Attention Do not merge the PR with the Petri nets (and the benchmark script)

@fit-alessandro-berti fit-alessandro-berti changed the title Perf: optimize petri net replay draft: Perf: optimize petri net replay Oct 4, 2023
@EduardoGoulart1
Copy link
Contributor Author

@Javert899 Is there anything missing on this PR? I took a look at the Dijkstra less memory method and it should be possible to rewrite the compiled semantics class to use some of the techniques there. This should make the alignment code cleaner as well as exposing this efficient representation to the end user

@EduardoGoulart1
Copy link
Contributor Author

EduardoGoulart1 commented Mar 6, 2024

@Javert899 any progress here? I guess now the code is outdated

@fit-alessandro-berti
Copy link
Contributor

@EduardoGoulart1 we will integrate more when our contribution level agreement will be approved by the Fraunhofer.

Until then, we will try to identify specific contributions with high impact and integrate giving due credits

@EduardoGoulart1
Copy link
Contributor Author

@fit-alessandro-berti Since there has been no action on this PR, do you think we should close it?

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