Code cleanup, README updates, interfaces for predictions and metric results - #20
Code cleanup, README updates, interfaces for predictions and metric results#20dtronmans wants to merge 6 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…less naive counting of number of classes using background
…ccuracy with different k
klemen1999
left a comment
klemen1999
left a comment
There was a problem hiding this comment.
At the moment I'm still a bit undecided about this new Predictions class because it technically closes down the range of supported tasks. Because if let's say I want to evaluate an OCR model the current Predictions doesn't support it so technically I'm breaking contract even though my custom parser and custom metrics that I registered can work with this custom message type. I'll think a bit more before I propose some changes.
Generally I'd like to keep luxonis-eval as a lightweight framework that doesn't enforce some contracts unless it really has to. And the interesting part is finding the right balance.
| report = make_report_table( | ||
| engine_name=engine_name, | ||
| result = EvaluationResult( | ||
| evaluator_name=self.evaluator_cfg.name, |
There was a problem hiding this comment.
self.evaluator_cfg.name is optional config item. So we should have a fallback since EvaluationResult expects string. We can have either empty string or if you can think of any other logical fallback string
| pct = (ms / total * 100.0) if total else 0.0 | ||
| return f"{ms:5.2f} ms | {pct:4.1f}%" | ||
| def _format_stage_latency(name: str, throughput: ThroughputResult) -> str: | ||
| ms = float(getattr(throughput, f"{name}_ms_per_sample")) |
| class ThroughputResult: | ||
| """End-to-end throughput and latency measurements.""" | ||
|
|
||
| elapsed_s: float | ||
| samples: int | ||
| samples_per_s: float | ||
| ms_per_sample: float | ||
| overhead_ms_per_sample: float | ||
| inference_ms_per_sample: float | ||
| parsing_ms_per_sample: float | ||
| metric_update_ms_per_sample: float | ||
| metric_compute_ms_per_sample: float | ||
|
|
There was a problem hiding this comment.
Just a note: This will likely completely change when we introduce proper benchmarking but yeah, we can have it like this for the v0.1.0 of lux-eval
| """Extract a semantic-segmentation mask from a prediction payload.""" | ||
| if hasattr(predictions, "getCvMask"): | ||
| mask = predictions.getCvMask() | ||
| elif hasattr(predictions, "getCvSegmentationMask"): | ||
| mask = predictions.getCvSegmentationMask() | ||
| else: |
There was a problem hiding this comment.
We shouldn't need this fallback right since we are locked to a dai-nodes (and dai) that only have one of these as valid
| @@ -0,0 +1,125 @@ | |||
| from dataclasses import dataclass | |||
| from typing import Mapping | |||
There was a problem hiding this comment.
Linter suggestion: Import from collections.abc instead: Mapping
2 participants
Purpose
Predictioncontainer instead of task-specific raw message typesEvaluationResult/ThroughputResultobjects instead of loose dictsparsers/utils/yolo.pymetrics/utils/segmentation.pySpecification
None / not applicable
Dependencies & Potential Impact
None / not applicable
Deployment Plan
None / not applicable
Testing & Validation
None / not applicable
AI Usage
Assisted-by: AGENT_NAME:MODEL_VERSION [TOOL1] [TOOL2]
Submitted code was reviewed by a human: YES/NO
The author is taking the responsibility for the contribution: YES/NO