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.

Sign up for GitHub

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

Jump to bottom

Document benchmarking #1053

Merged
merged 3 commits into from
Sep 24, 2024
Merged

Document benchmarking #1053

merged 3 commits into from
Sep 24, 2024

Conversation

umut-sahin
Copy link
Contributor

No description provided.

@umut-sahin umut-sahin self-assigned this Sep 18, 2024
@cla-bot cla-bot bot added the cla-signed label Sep 18, 2024
@umut-sahin umut-sahin requested a review from yuxizama September 18, 2024 08:04
yuxizama
yuxizama requested changes Sep 18, 2024
View reviewed changes
Copy link
Contributor

@yuxizama yuxizama left a comment

Choose a reason for hiding this comment

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

Some suggestions, thanks!

docs/dev/benchmarking.md Show resolved Hide resolved
docs/dev/benchmarking.md Outdated Show resolved Hide resolved
docs/dev/benchmarking.md Outdated Show resolved Hide resolved
docs/dev/benchmarking.md Outdated Show resolved Hide resolved
docs/dev/benchmarking.md Outdated Show resolved Hide resolved
docs/dev/benchmarking.md Outdated Show resolved Hide resolved
@umut-sahin umut-sahin requested a review from yuxizama September 19, 2024 10:54
yuxizama
yuxizama approved these changes Sep 19, 2024
View reviewed changes
Copy link
Contributor

@yuxizama yuxizama left a comment

Choose a reason for hiding this comment

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

Thank you so much @umut-sahin

bcm-at-zama
bcm-at-zama requested changes Sep 19, 2024
View reviewed changes
Copy link
Contributor

@bcm-at-zama bcm-at-zama left a comment

Choose a reason for hiding this comment

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

Thx @umut-sahin.

Should we have either another .md for the tests of examples, or an update to have explanations here for tests? I don't mean explaining pytest, but more, what's the philosophy with the classes, like what you had mentioned to me in #1040 (comment) (even if I don't fully agree here)

docs/SUMMARY.md Show resolved Hide resolved
docs/dev/benchmarking.md Show resolved Hide resolved
docs/dev/benchmarking.md Outdated Show resolved Hide resolved
docs/dev/benchmarking.md Outdated Show resolved Hide resolved
docs/dev/benchmarking.md Show resolved Hide resolved
@umut-sahin umut-sahin requested a review from yuxizama September 23, 2024 12:35
@umut-sahin
Copy link
Contributor Author

Benchmark list is removed and branch is rebased on main. Can I get another review, so that if it's okay, we can merge!

@bcm-at-zama
Copy link
Contributor

I'll review soon

@bcm-at-zama
Copy link
Contributor

Having a look

bcm-at-zama
bcm-at-zama requested changes Sep 24, 2024
View reviewed changes
Copy link
Contributor

@bcm-at-zama bcm-at-zama left a comment

Choose a reason for hiding this comment

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

Thanks @umut-sahin

Asking for a few last modifs, after the new test-notebooks. Also, having you've done this, I think it would be nice to ask @yuxizama to review the new .md. (That's why it's better to ask her to review only when the tech team is ok)

docs/SUMMARY.md Show resolved Hide resolved
docs/dev/benchmarking.md Outdated Show resolved Hide resolved
docs/dev/examples.md Outdated Show resolved Hide resolved
docs/dev/examples.md Outdated Show resolved Hide resolved
docs/dev/examples.md Outdated Show resolved Hide resolved
docs/dev/examples.md Outdated Show resolved Hide resolved
bcm-at-zama
bcm-at-zama approved these changes Sep 24, 2024
View reviewed changes
Copy link
Contributor

@bcm-at-zama bcm-at-zama left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @umut-sahin

@umut-sahin umut-sahin merged commit 52636e4 into main Sep 24, 2024
31 of 33 checks passed
@umut-sahin umut-sahin deleted the doc/benchmarking branch September 24, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuxizama yuxizama yuxizama approved these changes

@bcm-at-zama bcm-at-zama bcm-at-zama approved these changes

@BourgerieQuentin BourgerieQuentin Awaiting requested review from BourgerieQuentin

Assignees

@umut-sahin umut-sahin

Labels
cla-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@umut-sahin @bcm-at-zama @BourgerieQuentin @yuxizama