Skip to content

Conversation

@andrewkallai
Copy link

No description provided.

@vercel
Copy link

vercel bot commented Aug 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
llvm-ir-dataset-utils ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 25, 2025 0:55am

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

It doesn't seem like there's any script here to invoke this from the command line to actually run it? How do you envision this being used? Inside a Jupyter notebook?

If this is planned on being used through a CLI script, at least a simple one should probably land within this patch.

…ial jupyter notebook file to display results.
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

Mostly LGTM, one final comment.

Also, please take a look at the failing CI jobs.

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

Might be useful to read the Google Python style guide to see how good docstrings are written: https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings.

There are some reasonable examples in https://github.com/google/ml-compiler-opt/tree/main/compiler_opt/benchmark if you look through the individual python files.

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

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

LGTM.

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