Skip to content

order accuracy performance tools changes - #239

Draft
sainijit wants to merge 4 commits into
intel-retail:mainfrom
sainijit:main
Draft

order accuracy performance tools changes#239
sainijit wants to merge 4 commits into
intel-retail:mainfrom
sainijit:main

Conversation

@sainijit

Copy link
Copy Markdown
Contributor

PR Checklist

  • Added label to the Pull Request for easier discoverability and search
  • Commit Message meets guidelines as indicated in the URL https://github.com/intel-retail/performance-tools/blob/main/CONTRIBUTING.md
  • Every commit is a single defect fix and does not mix feature addition or changes
  • Unit Tests have been added for new changes
  • Updated Documentation as relevant to the changes
  • All commented code has been removed
  • If you've added a dependency, you've ensured license is compatible with repository license and clearly outlined the added dependency.
  • PR change contains code related to security
  • PR introduces changes that breaks compatibility with other modules (If YES, please provide details below)

What are you changing?

Issue this PR will close

close: #issue_number

Anything the reviewer should know when reviewing this PR?

Test Instructions if applicable

If the there are associated PRs in other repositories, please link them here (i.e. intel-retail/performance-tools )

TanmayeeSharvani22 and others added 2 commits July 23, 2026 11:38
…adata

- Fix _collect_metrics calling nonexistent stream_density.calculate_total_fps,
  which raised AttributeError on every run for the image-based dine-in
  workflow (no GStreamer pipeline logs exist for it). Skipped via hasattr()
  check instead of emitting a misleading warning each time.
- Add percentile/variance stats (mean, stddev, p50, p95, min, max) for
  latency and accuracy across all iterations, since a single mean hid
  significant run-to-run variance.
- Add per-image breakdown (order_id -> avg accuracy/latency) so multi-image
  iteration runs don't collapse into one aggregate number.
- Capture and stamp run_config (model name, target device, image max_size)
  into every results file by reading dine-in/.env and vlm_client.py, so
  results are self-describing without needing external tracking.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the Order Accuracy benchmark script output so benchmark artifacts are easier to compare across runs by embedding run configuration metadata, adding distribution-style statistics, and making exported filenames more descriptive.

Changes:

  • Adds run configuration stamping (e.g., model name and image max size) into exported results and incorporates the model name into result filenames.
  • Adds percentile-style summary stats (latency/accuracy) and a per-image (order_id) breakdown to worker result aggregation.
  • Avoids attempting an FPS calculation path that depends on a missing stream_density.calculate_total_fps API for the dine-in workflow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +79 to +84
"""
Snapshot the active model/precision/resolution configuration so every
results file is self-describing (which model this run actually measured).
Reads dine-in/.env for OVMS_MODEL_NAME and the image max_size baked into
vlm_client.py, without requiring the app to expose a new endpoint.
"""
Comment on lines +403 to +410
worker_results["per_image"] = {
oid: {
"count": len(items),
"avg_accuracy": sum(i.get("accuracy_score") or 0 for i in items) / len(items),
"avg_latency_ms": sum(i.get("total_latency_ms") or 0 for i in items) / len(items),
}
for oid, items in per_image.items()
}

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

benchmark-scripts/benchmark_order_accuracy.py:92

  • os.path.dirname(self.compose_files[0]) can be an empty string when the compose file is provided without a directory component (e.g. docker-compose.yml). In that case, env_path/vlm_client_path become relative to the current working directory rather than the compose file location. Consider normalizing empty dirname to ..
        try:
            compose_dir = os.path.dirname(self.compose_files[0]) if self.compose_files else "."
            env_path = os.path.join(compose_dir, ".env")

benchmark-scripts/benchmark_order_accuracy.py:408

  • Per-image averages currently treat missing accuracy_score/total_latency_ms values as 0 and still divide by the total number of items, which can silently skew results when metrics are absent for some iterations. It’s more accurate to compute averages over non-null samples and return None when there are no samples.
            oid: {
                "count": len(items),
                "avg_accuracy": sum(i.get("accuracy_score") or 0 for i in items) / len(items),
                "avg_latency_ms": sum(i.get("total_latency_ms") or 0 for i in items) / len(items),
            }

benchmark-scripts/benchmark_order_accuracy.py:683

  • model_name is read from an external .env file and then incorporated into output filenames with only / and space replaced. Other characters (quotes, :, \, newlines, etc.) can still produce invalid or hard-to-handle filenames across platforms and tools. Consider sanitizing to a conservative character set before building model_tag.
        model_tag = ""
        model_name = results.get("run_config", {}).get("model_name")
        if model_name:
            model_tag = "_" + model_name.replace("/", "-").replace(" ", "-")

benchmark-scripts/benchmark_order_accuracy.py:83

  • Docstring claims this reads dine-in/.env and vlm_client.py, but the implementation reads a .env file adjacent to the first compose file (compose_dir) and src/services/vlm_client.py under that directory. Updating the docstring avoids misleading readers about the expected paths.

This issue also appears in the following locations of the same file:

  • line 90
  • line 680
        Snapshot the active model/precision/resolution configuration so every
        results file is self-describing (which model this run actually measured).
        Reads dine-in/.env for OVMS_MODEL_NAME and the image max_size baked into
        vlm_client.py, without requiring the app to expose a new endpoint.

…ge averages

- Rewrite _capture_run_metadata docstring to match the fields actually
  exported (model_name, target_device, image_max_size).
- Average per-image accuracy/latency only over non-null samples so failed
  iterations no longer skew results toward 0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

benchmark-scripts/benchmark_order_accuracy.py:403

  • order_id can be present but None in worker_results["details"] (it’s populated via result.get("order_id")). Using d.get("order_id", "unknown") will then yield None (not the fallback), producing a None key in per_image which serializes to a surprising JSON key ("null") and is awkward for downstream consumers. Coerce falsy/None order IDs to a stable string key.
        for d in worker_results["details"]:
            oid = d.get("order_id", "unknown")
            per_image.setdefault(oid, []).append(d)
        worker_results["per_image"] = {

benchmark-scripts/benchmark_order_accuracy.py:690

  • model_tag only replaces / and spaces. Model identifiers often contain other filesystem-unfriendly characters (e.g., :, @, =, ,), which can make results export brittle across environments and harder to glob reliably. Consider sanitizing to a conservative character set (alnum, ., _, -) before embedding into filenames.
        model_tag = ""
        model_name = results.get("run_config", {}).get("model_name")
        if model_name:
            model_tag = "_" + model_name.replace("/", "-").replace(" ", "-")

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.

3 participants