-
Notifications
You must be signed in to change notification settings - Fork 6
Description
Follow-up items from code review of #113.
1. Benchmark total_images counter uses configured batch size instead of actual count
trapdata/antenna/benchmark.py:121 increments total_images += batch_size (the configured parameter) rather than the actual number of items in the batch. The last batch is typically partial, so this overcounts and corrupts the reported success rate.
Fix: total_images += batch_successful + batch_failed
ami-data-companion/trapdata/antenna/benchmark.py
Lines 116 to 123 in 50996ea
| # Count images in this batch | |
| batch_failed = len(batch["failed_items"]) | |
| # Successful items are those with reply_subjects that are not in failed_items | |
| batch_successful = len(batch["reply_subjects"]) | |
| total_images += batch_size | |
| total_successful_images += batch_successful | |
| total_failed_images += batch_failed |
2. ResultPoster.shutdown() can mask exceptions
If an exception propagates out of the batch loop in _process_job, the code jumps to the cleanup path without calling wait_for_all_posts() first. If pending_futures is non-empty at that point, shutdown() behavior may be unexpected. Consider wrapping the loop in try/finally that calls wait_for_all_posts() before shutdown().
ami-data-companion/trapdata/antenna/worker.py
Lines 165 to 167 in 50996ea
| ) | |
| continue | |
| crop = image_tensor[:, y1:y2, x1:x2] |
3. Removed pre-batch progress logs reduce debuggability
The PR replaced two per-batch logger.info calls ("Processing worker batch N (M images)" and "Total items processed so far: N") with a single end-of-batch summary log. If a crash occurs mid-batch during detection or classification, no progress is logged before the error, making it harder to diagnose which batch failed.
ami-data-companion/trapdata/antenna/worker.py
Lines 343 to 345 in 50996ea
| detections_for_terminal_classifier = detector_results | |
| detections_to_return = [] | |
4. load_time metric includes post-submission overhead
The timer resets after classification. The next iteration's load_time measurement therefore includes result-object construction, post_async() call, and the logger.info call, in addition to actual dataloader I/O. This inflates reported load times for batches after the first.
ami-data-companion/trapdata/antenna/worker.py
Lines 167 to 171 in 50996ea
| crop = image_tensor[:, y1:y2, x1:x2] | |
| crop_pil = to_pil(crop) | |
| crop_transformed = binary_transforms(crop_pil) | |
| binary_crops.append(crop_transformed) | |
| binary_valid_indices.append(idx) |