Update endpoints structure#2630
Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
This reverts commit 7d8ae1a.
| - config.yaml / config.yml (lowest priority) | ||
| scenario_dir: path to the scenario directory containing: | ||
| - config.yaml (scenario root) | ||
| - performance/run_1/results_summary.json (performance metrics) |
There was a problem hiding this comment.
It will not have run_1 folder in endpoints
There was a problem hiding this comment.
correct, run_1 is not needed
…tale results.json (#2631) The endpoints accuracy artifact was renamed from results.json (with a responses field) to accuracy/accuracy_results.json in mlcommons/endpoints#400, which also dropped the responses field entirely (per-sample text now lives in events.jsonl, not part of the submission). truncate_accuracy_log.py still looked for the old results.json/responses shape and would only log "missing" for every endpoints submission. Detect the endpoints layout via the scenario-root config.yaml and skip truncation, and remove the now-dead PERFORMANCE_ENDPOINTS_DIR/ACCURACY_ENDPOINTS_DIR constants left over from the loader.py refactor to ENDPOINTS_SCENARIO_DIR. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
| @@ -24,7 +24,9 @@ | |||
| "sample_logs") | |||
|
|
|||
| _RESULT_SUMMARY_FILE = "results_summary.json" | |||
There was a problem hiding this comment.
this should be result_summary.json instead of results_summary.json https://github.com/mlcommons/endpoints/blob/bf9d12b105a833c1bfeb3365efce091b6bc93a15/src/inference_endpoint/commands/benchmark/execute.py#L1565
| - config.yaml / config.yml (lowest priority) | ||
| scenario_dir: path to the scenario directory containing: | ||
| - config.yaml (scenario root) | ||
| - performance/run_1/results_summary.json (performance metrics) |
There was a problem hiding this comment.
correct, run_1 is not needed
| # Alternative JSON paths for endpoints keys that don't directly match the | ||
| # JSON structure | ||
| ENDPOINTS_JSON_ALT_PATHS = { | ||
| "result.total": "results.total", |
There was a problem hiding this comment.
should be
"result.total": ("results.total", "n_samples_completed"), because newly updated endpoint performance log is using n_samples_completed as a indicator of total sample run
| # JSON structure | ||
| ENDPOINTS_JSON_ALT_PATHS = { | ||
| "result.total": "results.total", | ||
| "result.failed": "results.failed", |
There was a problem hiding this comment.
should be "result.failed": ("results.failed", "n_samples_failed"), same reason as above
| ENDPOINTS_JSON_ALT_PATHS = { | ||
| "result.total": "results.total", | ||
| "result.failed": "results.failed", | ||
| "qps": "results.qps", |
There was a problem hiding this comment.
should be "qps": ("results.qps", "qps"),
| self.messages[qps_key] = [{"key": qps_key, "value": qps}] | ||
|
|
||
| # Expose accuracy scores stored in results.json | ||
| for result in results_data.get("accuracy_scores", {}).values(): |
There was a problem hiding this comment.
should be a list instead of a dictionary
for result in results_data.get("accuracy_scores", {}).values():
| for result in results_data.get("accuracy_scores", []): |
|
Tested this submission checker with a couple of internal valid log. Find a bug: currently since both accuracy and performance logs are generated together, submission checker will enforce n_samples_issued and n_samples_completed in performance/result_summary.json to be exactly the same size of the dataset. However, this enforcement should be in accuracy log instead of performance log. Performance log should be valid as long as min duration is > 600000 ms, and number of samples > performance dataset size. This is behavior is correct in legacy loadgen's result parsing path, but not correct in endpoint_parser.py |
4 participants
Partially address #2628