Skip to content

Conversation

@ani300
Copy link
Contributor

@ani300 ani300 commented Jun 30, 2025

Description of the change

This is an updated version of @andrea-fasoli 's refactor of my FP8 work, adding Paged Attention kernels as well as cleaning the code.

Related issues or PRs

#149

How to verify the PR

Code review (including math) is required.

Was the PR tested

  • I have added >=1 unit test(s) for every new method I have added (if that coverage is difficult, please briefly explain the reason)
  • I have ensured all unit tests pass

Checklist for passing CI/CD:

  • All commits are signed showing "Signed-off-by: Name <email@domain.com>" with git commit -signoff or equivalent
  • PR title and commit messages adhere to Conventional Commits
  • Contribution is formatted with tox -e fix
  • Contribution passes linting with tox -e lint
  • Contribution passes spellcheck with tox -e spellcheck
  • Contribution passes all unit tests with tox -e unit

Note: CI/CD performs unit tests on multiple versions of Python from a fresh install. There may be differences with your local environment and the test environment.

andrea-fasoli and others added 6 commits June 25, 2025 01:19
Signed-off-by: Andrea Fasoli <andrea.fasoli@ibm.com>
Signed-off-by: Andrea Fasoli <andrea.fasoli@ibm.com>
Signed-off-by: Andrea Fasoli <andrea.fasoli@ibm.com>
Signed-off-by: Andrea Fasoli <andrea.fasoli@ibm.com>
Signed-off-by: Andrea Fasoli <andrea.fasoli@ibm.com>
Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
ani300 added 2 commits June 30, 2025 19:57
Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
@andrea-fasoli
Copy link
Collaborator

this PR needs minimal changes and in my opinion it is good to go. FP8 addons are an experimental feature which may require further validation of math and model outputs, but they don't interact with other parts of FMS-MO, so won't break existing code. Only exception is the additional import of torchao, only needed for fp8, which is being added to the build as optional requirement. @tharapalanivel @chichun-charlie-liu please check that this is done appropriately.

@ani300 we have barebone unit tests for other addons, that check op registration in the torch namespace and output shape validation: https://github.com/foundation-model-stack/fms-model-optimizer/blob/main/tests/aiu_addons/test_gptq_addon.py
Could we add the same for FP8? They are very basic, we have plans to expand them for full output validation against expectations, but for now something like this will do.

ani300 added 2 commits July 1, 2025 17:18
Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
Copy link
Collaborator

@andrea-fasoli andrea-fasoli left a comment

Choose a reason for hiding this comment

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

this PR looks ready to go

@BrandonGroth
Copy link
Collaborator

Now that ibm-fms is an optional package, we will need to add guards to each file that imports fms similar to the other aiu_addon files.

Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
@ani300
Copy link
Contributor Author

ani300 commented Jul 2, 2025

@BrandonGroth done

ani300 added 3 commits July 2, 2025 16:55
Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
Signed-off-by: Antoni Viros i Martin <aviros@ibm.com>
@ani300 ani300 merged commit 6c54b37 into foundation-model-stack:main Jul 3, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants