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

Reviewed MaxText README.md #590

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mikegre-google
Copy link
Collaborator

I updated the MaxText/README.md file for clarity and for it to follow the devsite style guide (mostly).

Copy link
Collaborator

@rwitten rwitten left a comment

Choose a reason for hiding this comment

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

Looks amazing! There are some nits, please take a look.

README.md Outdated
@@ -19,13 +19,22 @@

# Overview

MaxText is a **high performance**, **highly scalable**, **open-source** LLM written in pure Python/Jax and targeting Google Cloud TPUs and GPUs for **training** and **inference**. MaxText achieves [high MFUs](#runtime-performance-results) and scales from single host to very large clusters while staying simple and "optimization-free" thanks to the power of Jax and the XLA compiler.
MaxText is a set of open source reference implementations that are
Copy link
Collaborator

Choose a reason for hiding this comment

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

When you write, do you view your output in markdown? I don't mind your line breaks assuming they aren't showing up externally unexpectedly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(I did view it here and it looked good but I just want to make sure you own the Markdown nonsense: https://github.com/mikegre-google/maxtext/tree/main)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did view the rendered output and it looked OK, but if you usually don't put in the extra line breaks, I'm happy to remove them. Either way I will own any markdown issues.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@mikegre-google
Copy link
Collaborator Author

I addressed the feedback and removed the extra new lines from README.md

additional readme updates
@mikegre-google
Copy link
Collaborator Author

I squashed my commits and pushed the changes to a new branch. I got a message saying "warning you are about to push to the main branch" and, while I'm pretty sure I can't push to this repo's main branch, I didn't want to take any chances of messing anything up since my forked repo has not been synced with this repo. Please let me know if there is anything else I need to do. Michelle Yoo told me that after I squash my commits, and my changes pass whatever testing is done, a CL will be created. but I don't know if that CL will be assigned to me or if I get notified when that happens. Sorry for the newbie questions. Once I go through this process, I won't have to ask all these questions again. Thanks for your understanding.

additional readme updates

README updates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants