Skip to content

Conversation

@joerunde
Copy link

@joerunde joerunde commented Aug 6, 2025

Description of the change

This updates a check in fp8_linear_type to ensure the module name is not none before checking it against a string.

See this todo here: https://github.com/foundation-model-stack/foundation-model-stack/blob/f4d8d1f062cf57d8258b52b18db7239722731553/fms/modules/attention.py#L779-L780

NB: I am not an expert here and don't know if this is correct

Related issues or PRs

How to verify the PR

Load the model ibm-ai-platform/micro-g3.3-8b-instruct-1b-FP8 with TP>1 using fms

Was the PR tested

Coverage is difficult because I don't know what the correct behavior should be, but this change at least gets around the error about checking None with the in operator

  • 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 protected]>" 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.

@joerunde joerunde changed the title 🐛 fix name check to suport TP bugfix: fix name check to suport TP Aug 6, 2025
@joerunde joerunde changed the title bugfix: fix name check to suport TP fix: name check to support TP Aug 6, 2025
@github-actions github-actions bot added the fix label Aug 6, 2025
@ani300 ani300 mentioned this pull request Aug 6, 2025
8 tasks
@joerunde
Copy link
Author

joerunde commented Aug 6, 2025

@joerunde joerunde closed this Aug 6, 2025
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.

1 participant