Skip to content

🔧 Add argument validations #38

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

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

Conversation

gkumbhat
Copy link
Contributor

@gkumbhat gkumbhat commented Apr 25, 2025

Changes

  1. While running the generate_metrics script, I noticed a few issues, so creating this PR to add some validations.
  2. Add validation sharegpt_path, otherwise, args.sharegpt_path will return None which is causing exception at https://github.com/foundation-model-stack/aiu-fms-testing-utils/blob/main/aiu_fms_testing_utils/utils/__init__.py#L89

Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
@@ -27,6 +27,7 @@
type=str,
default=None,
help="The model variant (configuration) to benchmark. E.g. 7b, 13b, 70b.",
required=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

you need either model_variant or model_path, but you can can have architecture and model_path without variant for example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

variant seems to be required for this script, since it uses this information for creating file name prefixes etc: https://github.com/foundation-model-stack/aiu-fms-testing-utils/pull/38/files#diff-f2d7f13363f393d17bef3e70f84983fd1283ac80f0a826e92596151c753c030cL111

So I thought of making it explicit.

@gkumbhat gkumbhat force-pushed the minor_validations branch 2 times, most recently from 0a3641c to b18373e Compare April 28, 2025 20:35
Signed-off-by: gkumbhat <kumbhat.gaurav@gmail.com>
@gkumbhat gkumbhat force-pushed the minor_validations branch from b18373e to f01056e Compare April 28, 2025 20:39
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