fix(robot-cityscapes-synthia): fix three Sedna interface crashes blocking all pipeline runs#567
Open
rajdhruvsingh wants to merge 5 commits into
Conversation
Adds parallel execution of test cases to address the unbearable time overhead when testing multiple hyperparameter groups sequentially (issue 8). Changes: - testcasecontroller.py: add parallel/max_workers params to run_testcases(), add _run_testcases_sequential() and _run_testcases_parallel() using ThreadPoolExecutor - testcase.py: fix race condition in _get_output_dir() using os.makedirs(exist_ok=True) instead of os.path.exists() loop - benchmarkingjob.py: add parallel and max_workers config fields - docs/proposals/chore/parallel_testcase_execution.md: RFC proposing phased approach (ThreadPoolExecutor now, Sandbox Engine future per PR 526) ThreadPoolExecutor chosen over ProcessPoolExecutor to avoid pickling issues with ML model objects. Parallel execution is opt-in via benchmarkingjob.yaml (parallel: false by default) for full backward compatibility. Signed-off-by: dhruvraj319 <dhruv.raj319@gmail.com>
- Remove manual worker count default to avoid OOM on large test suites — let ThreadPoolExecutor use its own safe default - Fix type annotation: max_workers: int | None = None - Add YAML string coercion for parallel and max_workers params to handle parallel: 'false' string values correctly - Deep copy test cases in parallel mode to prevent race conditions from shared test_env and dataset state Signed-off-by: dhruvraj319 <dhruv.raj319@gmail.com>
…multiedge_inference_bench - fix(447): guard np.nonzero()[0][0] in rank_1/2/5.py to skip queries with no gallery match instead of raising IndexError - fix(447): replace RuntimeError in mAP.py with LOGGER.warning + return 0.0 so the pipeline degrades gracefully on empty query sets - fix(449): add requirements.txt with all missing dependencies (loguru, motmetrics, mmcv, opencv-python) and version bounds (motmetrics>=1.2.0, pandas<2.0) - fix(449): replace Docker-only absolute workspace path /ianvs/... with relative path ./workspace/... in tracking_job.yaml and reid_job.yaml - fix(450): move top-level 'import onnx' to lazy import inside _partition() so non-partitioning users (MOT17 example) are not forced to install onnx Signed-off-by: dhruvraj319 <dhruv.raj319@gmail.com>
kubeedge-bot
added
kind/bug
Collaborator
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rajdhruvsingh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubeedge-bot
added
the
size/XL
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces parallel test case execution using ThreadPoolExecutor to reduce benchmarking time, fixes a directory creation race condition, and updates various examples, requirements, and datasets for better robustness. The review feedback highlights critical parameter order mismatches in the mAP and rank_* metric signatures, which swap ground truth and predictions. Additionally, the feedback points out a correctness issue in the CMC calculation denominator for rank_1, rank_2, and rank_5, where skipped queries with no gallery matches are incorrectly included in the total count, artificially lowering the scores.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
kubeedge-bot
added
the
do-not-merge/invalid-commit-message
rajdhruvsingh
force-pushed
the
fix/robot-cityscapes-synthia-runtime-crashes
branch
from
b06e27a to
2cc9cf5
Compare
kubeedge-bot
removed
the
do-not-merge/invalid-commit-message
- fix(447): guard np.nonzero()[0][0] in rank_1/2/5.py to skip queries with no gallery match instead of raising IndexError - fix(447): fix CMC denominator — divide by valid_queries (queries with at least one gallery match) instead of m (all queries), fixing artificial underestimation of rank-1/2/5 scores - fix(447): replace RuntimeError in mAP.py with LOGGER.warning + return 0.0 so the pipeline degrades gracefully on empty query sets - fix(449): add requirements.txt with all missing dependencies (loguru, motmetrics, mmcv, opencv-python) and version bounds (motmetrics>=1.2.0, pandas<2.0) - fix(449): replace Docker-only absolute workspace path /ianvs/... with relative path ./workspace/... in tracking_job.yaml and reid_job.yaml - fix(450): move top-level 'import onnx' to lazy import inside _partition() so non-partitioning users (MOT17 example) are not forced to install onnx Signed-off-by: dhruvraj319 <dhruv.raj319@gmail.com>
…ask_allocation and cityscapes dataset Bug 1: TaskAllocationByOrigin.__call__() expected task_extractor as a parameter but Sedna passes it to __init__() and calls method(samples=samples). Moved task_extractor to __init__, removed it from __call__ signature. Bug 2: CityscapesSegmentation.__init__() crashed with 'NoneType has no len()' when data is a BaseDataSource. Added explicit hasattr(data, 'x') guard, None check on data.y for inference phase, and clear ValueError when data=None. Bug 3: Bare truthiness checks on numpy arrays (data.y from Sedna BaseDataSource) raised 'ValueError: truth value of array is ambiguous'. Replaced all such checks with isinstance() and hasattr() guards throughout. Fixes 473 Related to 230 Signed-off-by: dhruvraj319 <dhruv.raj319@gmail.com>
rajdhruvsingh
force-pushed
the
fix/robot-cityscapes-synthia-runtime-crashes
branch
from
2cc9cf5 to
1fe9479
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
/kind failing-test
What this PR does / why we need it:
Fixes three runtime crashes that blocked every training and evaluation run of
the robot-cityscapes-synthia lifelong learning example when using Sedna 0.6.0.1.
Bug 1 — task_allocation_by_domain.py: wrong call signature
Sedna instantiates TaskAllocationByOrigin(task_extractor=..., **kwargs) then
calls method(samples=samples). The old call expected task_extractor as a
positional argument and crashed immediately. Moved task_extractor to init
and stored as self.task_extractor.
Bug 2 — cityscapes.py: NoneType has no len() during inference
data.y is None when Sedna runs inference (no ground-truth labels available).
The old code assigned self.labels[split] = data.y unconditionally, then later
called self.labels[split][index].rstrip() which crashed. Added an explicit
data is None guard and a safe data.y is not None check before accessing labels.
Bug 3 — cityscapes.py: ValueError: truth value of array is ambiguous
data.y from Sedna is a numpy array. Bare truthiness checks like if data: or
data else None on a multi-element numpy array raise this error. Replaced all
such checks with isinstance() and hasattr() guards throughout init.
All three fixes verified with an isolated mock test against Sedna 0.6.0.1
BaseDataSource, covering train (with labels), inference (labels=None), and
numpy array label inputs.
Which issue(s) this PR fixes:
Fixes #473
Related to #230