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

Add blog post about PTPC FP8 on ROCm #38

Closed
wants to merge 11 commits into from

Conversation

tanpinsiang
Copy link
Contributor

Blog post about for PTPC-FP8 Quantization on ROCm
Author: AMD and Embedded LLM

@hmellor
Copy link
Member

hmellor commented Mar 10, 2025

3 things:

  • Please make the DCO check pass?
  • Could you refrain from using bold text in headings?
  • Is there a deadline for getting this blog published or would you like it published ASAP?

@tjtanaa
Copy link
Contributor

tjtanaa commented Mar 13, 2025

3 things:

  • Please make the DCO check pass?
  • Could you refrain from using bold text in headings?
  • Is there a deadline for getting this blog published or would you like it published ASAP?

@hmellor

  • Please make the DCO check pass?
    • Thank you we have fixed it.
  • Could you refrain from using bold text in headings?
    • Thank you for the feedback. I have fixed this to use the Markdown header.
  • Is there a deadline for getting this blog published or would you like it published ASAP?
    • We would like to get it published ASAP. Thank you.

Copy link
Member

@hmellor hmellor left a comment

Choose a reason for hiding this comment

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

Overall this looks fine to me, just a few comments about formatting that might help the readability and polish!

The title is quite long, this might not look nice in social previews and it looks a little jarring on the homepage (particularly on mobile)

image

Using LaTeX for math formulae might be nice (i.e. in the Understanding the Diagram section)

Comment on lines 145 to 147
<img align="right" src="/assets/figures/ptpc/PerplexityBits.png" alt="bits and byte perplexity" width="50%" height="50%">

<img align="right" src="/assets/figures/ptpc/Perplexitywords.png" alt="Word Perplexity Comparison" width="50%" height="50%">
Copy link
Member

Choose a reason for hiding this comment

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

Ditto on mobile

Copy link
Member

Choose a reason for hiding this comment

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

For more context, it looks like this:

image

  1. They will be very small on mobile
  2. They are different heights

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed now.

@tjtanaa
Copy link
Contributor

tjtanaa commented Mar 17, 2025

@hmellor Do you know what is the syntax for rendering the LaTeX in this framework?
We have also revised the title.

@hmellor hmellor mentioned this pull request Mar 17, 2025
@hmellor
Copy link
Member

hmellor commented Mar 17, 2025

Thanks for making those changes.

I looked into how to use LaTeX math and found that it wasn't actually possible initially. I have just merged a PR which enables this!

You can see the README for full instructions but you simply add math: true to the frontmatter and then use standard $math$ for linline math and $$math$$ for block math

@tjtanaa
Copy link
Contributor

tjtanaa commented Mar 18, 2025

@hmellor Thank you. The LaTeX rendering is working. We have followed your suggestion to render math formulae with LaTeX.

Copy link
Member

@hmellor hmellor left a comment

Choose a reason for hiding this comment

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

LGTM!

One thing I noticed is that the images you've scaled down aren't center aligning. If you confirm that this doesn't bother you, I'll merge!

@hmellor
Copy link
Member

hmellor commented Mar 19, 2025

@tanpinsiang can you please make sure the DCO check passes, it was passing before the most recent commit

tanpinsiang and others added 11 commits March 19, 2025 16:20
Signed-off-by: tanpinsiang <[email protected]>
Signed-off-by: tanpinsiang <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tanpinsiang <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tanpinsiang <[email protected]>
Follow up to vllm-project/vllm#14556.

Signed-off-by: Harry Mellor <[email protected]>
Signed-off-by: tanpinsiang <[email protected]>
Signed-off-by: Harry Mellor <[email protected]>
Signed-off-by: tanpinsiang <[email protected]>
Bumps [json](https://github.com/ruby/json) from 2.10.1 to 2.10.2.
- [Release notes](https://github.com/ruby/json/releases)
- [Changelog](https://github.com/ruby/json/blob/master/CHANGES.md)
- [Commits](ruby/json@v2.10.1...v2.10.2)

---
updated-dependencies:
- dependency-name: json
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: tanpinsiang <[email protected]>
Signed-off-by: tjtanaa <[email protected]>
Signed-off-by: tanpinsiang <[email protected]>
@tjtanaa
Copy link
Contributor

tjtanaa commented Mar 19, 2025

@hmellor it is fixed

@hmellor
Copy link
Member

hmellor commented Mar 19, 2025

Looks like something went wrong with the merge, lots of stuff from main is appearing in the diff

@tjtanaa
Copy link
Contributor

tjtanaa commented Mar 19, 2025

@hmellor we will create a new PR with a new branch. I followed the steps recommended by Github, found in after I click the following icon
image

It seems that messed up everything. ><

@hmellor
Copy link
Member

hmellor commented Mar 19, 2025

Thanks, sorry for the hassle. Yeah I've had trouble in the past with the DCO checks when trying to fix a branch with merge commits

@tjtanaa
Copy link
Contributor

tjtanaa commented Mar 19, 2025

@hmellor #45 This is the new PR.

@hmellor hmellor closed this Mar 20, 2025
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