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

Part of #3839 new benchmarks to output performance graph format #3894

Conversation

ajpotts
Copy link
Contributor

@ajpotts ajpotts commented Nov 14, 2024

This PR adds a script benchmark_v2/reformat_benchmark_results.py to convert the benchmark results from json to a format that matches the chapel team viz. It only processes this for argsort, coargsort, and aggregate. The other benchmarks will be added in a future PR.

Part of #3839 new benchmarks to output performance graph format

@ajpotts ajpotts changed the title 3839 new benchmarks to output performance graph format Part of #3839 new benchmarks to output performance graph format Nov 14, 2024
@ajpotts ajpotts force-pushed the 3839-new-benchmarks-to-output-performance-graph-format branch 7 times, most recently from 850e8c8 to 2f142dc Compare November 15, 2024 17:14
@ajpotts ajpotts marked this pull request as ready for review November 15, 2024 17:15
Copy link
Contributor

@drculhane drculhane left a comment

Choose a reason for hiding this comment

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

I've made a couple of comments in the code. Other than that, (1) there's a lot in here, and while the code is straightforward, I'm like a member of a doctoral defense committee -- I just don't know this stuff well enough to confirm it, and I'm not able to generate test data to check it, and there aren't any tests included, however (2a) this is a utility to be used on the output of arkouda, as I understand it, so tests aren't necessarily needed, and (2b) the code is well-written with clear intent. In my judgment, it merits approval, although I'd appreciate some feedback on my comments below, and a walkthrough in our next team meeting, if that's feasible, would be nice.

@ajpotts ajpotts force-pushed the 3839-new-benchmarks-to-output-performance-graph-format branch from 0069e0c to e60ebde Compare November 21, 2024 20:43
Copy link
Contributor

@jaketrookman jaketrookman 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, great work

@ajpotts ajpotts added this pull request to the merge queue Dec 3, 2024
Merged via the queue into Bears-R-Us:master with commit 40d1fd8 Dec 3, 2024
11 checks passed
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.

3 participants