-
Notifications
You must be signed in to change notification settings - Fork 13
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 benchmarks test #56
Conversation
976e576
to
c29de03
Compare
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.
Looks good!
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.
Looking deeper into the functions being benchmarked, aren't they executing much more than just the fhe execution? Perhaps we should see lower level functions to benchmark
I don't think so, because otherwise we could simply use tfhe-rs benchmarks. For example, right now we realise there is an issue so it's useful ;) |
I would agree if the functions we were benchmarking were fhevm function, but what's currently being benchmarked are test functions that never runs when executing the fhevm, so the numbers can be misleading due to an error in the test code, or an update to the test functions. |
The ultimate goal is just to have a rough idea of timings for estimate gas prices. I think it's useful to have these benchmarks. I can close the PR since I get the the values for update gas prices, but I'm pretty sure it will be useful in 3 or 6 months to reestimate gas prices. |
My only fear is that it's confused with fhevm benchmarks while it's a little bit more than that |
Would you prefer I rename it to "gas_estimation" ? |
Yes, I guess we can merge it if it's "gas estimation" in the tests and Makefile |
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.
I think you forgot to rename the file as well
920e027
to
f7d2add
Compare
f7d2add
to
22edf6e
Compare
No description provided.