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

Revert Summary Metrics and Expand Test Coverage to Stabilize Nightly/Main CI #58

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

markurtz
Copy link
Member

Summary:

Reverts the summary metrics logic in src/guidellm/core/report.py and src/guidellm/core/result.py that was landed due to failing tests. Additionally, test cases are expanded to ensure full coverage of these changes and to stabilize the nightly and main CI pipelines.

Details:

  • Replaced direct token statistics (prompt_token, output_token) with distribution-based calculations (prompt_token_distribution, output_token_distribution).
  • Modified percentile handling for request latency, time-to-first-token (TTFT), and inter-token latency (ITL) to improve performance summary accuracy.
  • Removed computed_field annotations for several properties in src/guidellm/core/result.py.
  • Updated tests in tests/unit/core/test_report.py from @pytest.mark.regression to @pytest.mark.sanity to better align with the testing standards.

Test Plan:

  • Unit tests have been added/updated to verify:
    • Correctness of the refactored token statistics and distribution calculations.
    • Accurate summary report generation for benchmarks.
    • Full compatibility with existing functionality.
  • Verified passing CI/CD pipeline, ensuring no regressions.

@markurtz markurtz self-assigned this Sep 11, 2024
Copy link
Contributor

@parfeniukink parfeniukink left a comment

Choose a reason for hiding this comment

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

Looks good

@markurtz markurtz merged commit 474ad29 into main Sep 11, 2024
9 checks passed
@markurtz markurtz deleted the main-fixes branch September 11, 2024 14:08
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