-
Notifications
You must be signed in to change notification settings - Fork 158
Changes to run bop_toolkit on industrial #216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adapts the bop_toolkit to work with industrial datasets by simplifying the codebase and removing support for multiple sensors and modalities per dataset. The changes primarily focus on simplifying dataset parameter handling and removing HOT3D-specific multi-sensor code.
Changes:
- Simplified dataset parameter handling by removing multi-sensor/multi-modality support
- Added support for "industrial" dataset with its specific configuration
- Removed dataset-specific configurations for "xyzibd" and "ipd" datasets
- Cleaned up logging methods, JSON compression support, and test code
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Removed ROS2/Ament package data configuration |
| scripts/vis_object_symmetries.py | Changed default dataset and improved camera parameter retrieval |
| scripts/vis_gt_poses.py | Simplified to remove multi-sensor support, standardized to single annotation format |
| scripts/vis_est_poses.py | Removed HOT3D-specific checks and multi-sensor logic |
| scripts/eval_calc_scores.py | Removed xyzibd-specific configuration and simplified scene parameter calls |
| scripts/eval_calc_errors_gpu.py | Removed dataset-specific configurations and simplified API calls |
| scripts/eval_calc_errors.py | Added industrial dataset config and removed xyzibd-specific settings |
| scripts/eval_bop24_pose.py | Removed custom threshold units and dataset-specific configurations |
| scripts/eval_bop22_coco.py | Simplified scene_tpaths_keys API call |
| scripts/eval_bop19_pose.py | Added industrial dataset configuration |
| scripts/enumerate_test_targets.py | Simplified to remove multi-sensor logic, added commented code |
| scripts/create_pose_results_file_from_gt.py | Fixed logic bug in result generation |
| scripts/create_coco_results_file_from_gt.py | Simplified scene_tpaths_keys call and changed print statement |
| scripts/calc_model_info.py | Fixed 3D bounding box calculation bug |
| scripts/calc_gt_masks.py | Simplified by removing multi-sensor parameter handling |
| scripts/calc_gt_info.py | Removed multi-sensor parameters |
| scripts/calc_gt_distribution.py | Simplified by removing visualization parameters and multi-sensor support |
| scripts/calc_gt_coco.py | Changed datetime call and simplified parameter handling |
| requirements.txt | Fixed Cython version constraint and removed Pillow upper bound |
| docs/bop_datasets_format.md | Removed scene_gt_coco.json from format documentation |
| bop_toolkit_lib/visualization.py | Removed depth difference statistics |
| bop_toolkit_lib/tests/test_misc.py | Removed Precomputer test |
| bop_toolkit_lib/tests/eval_bop22_coco_test.py | Simplified test structure and removed compression support |
| bop_toolkit_lib/renderer_batch.py | Changed worker script path |
| bop_toolkit_lib/misc.py | Fixed Precomputer cache update bug |
| bop_toolkit_lib/inout.py | Removed gzip compression support and simplified JSON I/O |
| bop_toolkit_lib/dataset_params.py | Major refactor removing multi-sensor support and adding industrial dataset |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Refer to the README.md for installation instructions. | ||
| """) | ||
|
|
||
| logger.warn("""Missing hand_tracking_toolkit dependency, |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logger.warn() method is deprecated in Python. Use logger.warning() instead.
| logger.warn("""Missing hand_tracking_toolkit dependency, | |
| logger.warning("""Missing hand_tracking_toolkit dependency, |
| ref_pt = map(float, model["pts"].min(axis=0).flatten()) | ||
| size = map(float, (model["pts"].max(axis=0) - ref_pt).flatten()) |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Python 3, map() returns an iterator, not a list. After using ref_pt in line 37 for the calculation, it's consumed. When trying to access ref_pt[0], ref_pt[1], ref_pt[2] in lines 43-45, this will fail with a TypeError. Convert the map objects to lists: ref_pt = list(map(float, model['pts'].min(axis=0).flatten())) and size = list(map(float, (model['pts'].max(axis=0) - ref_pt).flatten())).
| ref_pt = map(float, model["pts"].min(axis=0).flatten()) | |
| size = map(float, (model["pts"].max(axis=0) - ref_pt).flatten()) | |
| ref_pt = list(map(float, model["pts"].min(axis=0).flatten())) | |
| size = list(map(float, (model["pts"].max(axis=0) - ref_pt).flatten())) |
| # test_targets_lines = [] | ||
| # for test_target in test_targets: | ||
| # test_targets_lines.append( | ||
| # '- {{scene_id: {}, im_id: {}, obj_id: {}, inst_count: {}}}'.format( | ||
| # test_target['scene_id'], test_target['im_id'], test_target['obj_id'], | ||
| # test_target['inst_count'])) | ||
|
|
||
| # Save the test targets, | ||
| test_targets_path = os.path.join(dp_split["base_path"], p["test_targets_filename"]) | ||
| # with open(test_targets_path, 'w') as f: | ||
| # f.write('\n'.join(test_targets_lines)) |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Large blocks of commented-out code should be removed. If this code might be needed in the future, it can be retrieved from version control. Dead code reduces maintainability and creates confusion about what is actually being executed.
| # test_targets_lines = [] | |
| # for test_target in test_targets: | |
| # test_targets_lines.append( | |
| # '- {{scene_id: {}, im_id: {}, obj_id: {}, inst_count: {}}}'.format( | |
| # test_target['scene_id'], test_target['im_id'], test_target['obj_id'], | |
| # test_target['inst_count'])) | |
| # Save the test targets, | |
| test_targets_path = os.path.join(dp_split["base_path"], p["test_targets_filename"]) | |
| # with open(test_targets_path, 'w') as f: | |
| # f.write('\n'.join(test_targets_lines)) | |
| # Save the test targets, | |
| test_targets_path = os.path.join(dp_split["base_path"], p["test_targets_filename"]) |
| "year": datetime.date.today().year, | ||
| "contributor": "", | ||
| "date_created": datetime.datetime.now(datetime.timezone.utc).isoformat(" "), | ||
| "date_created": datetime.datetime.utcnow().isoformat(" "), |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
datetime.datetime.utcnow() is deprecated as of Python 3.12. Use datetime.datetime.now(datetime.timezone.utc) instead for timezone-aware UTC timestamps.
| "date_created": datetime.datetime.utcnow().isoformat(" "), | |
| "date_created": datetime.datetime.now(datetime.timezone.utc).isoformat(" "), |
| cmd = [ | ||
| "python", | ||
| "bop_toolkit_lib/call_vsd_worker.py", | ||
| "external/bop_toolkit/bop_toolkit_lib/call_vsd_worker.py", |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The path to the worker script has been changed from bop_toolkit_lib/call_vsd_worker.py to external/bop_toolkit/bop_toolkit_lib/call_vsd_worker.py. This appears to be an environment-specific path that may not work for all users. The path should be relative to the repository root or use __file__ to construct a path relative to the current module.
| "handal": [26, 35, 36, 37, 38, 39, 40], | ||
| "ipd": [8, 14, 18, 19, 20], | ||
| "xyzibd": [1, 2, 5, 8, 9, 11, 12, 16, 17] | ||
| "industrial": [1,3,5] |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent spacing in list formatting. Use consistent spacing after commas: [1, 3, 5].
| "industrial": [1,3,5] | |
| "industrial": [1, 3, 5] |
| "quest3": {"gray1": ".jpg", "gray2": ".jpg"} | ||
| "rgb": ".jpg", | ||
| "gray1": ".jpg", | ||
| "gray2": "jpg", |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing dot prefix in file extension. Should be '.jpg' to match the pattern used for 'rgb' and 'gray1' extensions in lines 442-443.
| "gray2": "jpg", | |
| "gray2": ".jpg", |
| p["elev_range"] = (-0.5 * math.pi, 0.5 * math.pi) | ||
|
|
||
| elif dataset_name == "industrial": | ||
| p["scene_ids"] = [1,2,3,4,5,6,7,8] |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent spacing in list formatting. Use consistent spacing after commas: [1, 2, 3, 4, 5, 6, 7, 8].
| p["scene_ids"] = [1,2,3,4,5,6,7,8] | |
| p["scene_ids"] = [1, 2, 3, 4, 5, 6, 7, 8] |
| sensor_modalities_have_separate_annotations = {"aria": True, "quest3": True} | ||
| p["im_modalities"] = {"aria": ["rgb", "gray1", "gray2"], "quest3": ["gray1", "gray2"]} | ||
| modalities_have_separate_annotations = True | ||
| p["im_modalities"] = ["rgb","gray1","gray2"] |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent spacing in list formatting. Add spaces after commas: ['rgb', 'gray1', 'gray2'].
| p["im_modalities"] = ["rgb","gray1","gray2"] | |
| p["im_modalities"] = ["rgb", "gray1", "gray2"] |
| from tqdm import tqdm | ||
| from bop_toolkit_lib import inout | ||
|
|
||
| # |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty comment line should be removed.
| # |
No description provided.