-
Notifications
You must be signed in to change notification settings - Fork 0
Position filtering 2 #81
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: main
Are you sure you want to change the base?
Conversation
…ing in changes made to position refiners
…finer's constructor
…ored classes, added new datasets
…e testing utilities
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 PR implements FIR (Finite Impulse Response) filtering for noise reduction in robot position tracking from vision data. The implementation includes a new filter module, comprehensive unit tests validating MSE reduction, and integration into the position refinement pipeline with support for both friendly and enemy robot tracking.
Key Changes:
- New FIR filter implementation with configurable parameters for position smoothing
- Unit tests demonstrating filter effectiveness across 6 robots using real simulation data
- Updated PositionRefiner API to accept team color and robot count parameters for filter initialization
- Added visualization and analysis dependencies (scipy, seaborn, pyqtgraph, pyside6)
Reviewed changes
Copilot reviewed 17 out of 24 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
utama_core/run/refiners/filters.py |
New FIR filter implementation with position and orientation filtering support |
utama_core/tests/refiners/noise_filter_test.py |
Comprehensive unit tests validating MSE reduction for x, y, and vector positions |
utama_core/run/refiners/position.py |
Integration of filters into position refiners (not shown in diff) |
utama_core/tests/refiners/position_unit_test.py |
Updated test signatures to match new PositionRefiner constructor |
utama_core/tests/refiners/position_refiner_integration_test.py |
Updated integration test with new PositionRefiner parameters |
utama_core/run/strategy_runner.py |
Updated PositionRefiner instantiation with team and robot count parameters |
utama_core/entities/data/vision.py |
Added Gaussian noise injection method for testing |
pixi.toml |
Added scipy, seaborn, pyqtgraph, and pyside6 dependencies |
vision_data/*.csv |
Test data files for filter validation |
vision_data/.jupyter/* |
Jupyter workspace configuration (should not be committed) |
main.py |
Changed mode from "rsim" to "grsim" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Manually adds noise; do not use a map, Python maps are lazy. | ||
| # for robot in combined_vision_data.yellow_robots: | ||
| # robot.add_gaussian_noise() |
Copilot
AI
Dec 9, 2025
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.
This comment appears to contain commented-out code.
| # robot.add_gaussian_noise() |
| # # TH_COL: b_robot.orientation | ||
| # # }) | ||
|
|
||
| # self.data_collected += 1 |
Copilot
AI
Dec 9, 2025
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.
This comment appears to contain commented-out code.
| # self.data_collected += 1 |
| # # for b_robot in combined_vision_data.blue_robots: | ||
| # # writer.writerow({ | ||
| # # TS_COL: combined_vision_data.ts, | ||
| # # ID_COL: b_robot.id, | ||
| # # COLOR_COL: "blue", | ||
| # # X_COL: b_robot.x, | ||
| # # Y_COL: b_robot.y, | ||
| # # TH_COL: b_robot.orientation | ||
| # # }) | ||
|
|
||
| # self.data_collected += 1 | ||
|
|
||
| # For live testing: | ||
| # try: | ||
| # current_coord = np.array(( | ||
| # filtered_vision_data.yellow_robots[2].x, | ||
| # filtered_vision_data.yellow_robots[2].y | ||
| # )) | ||
| # if not np.isnan(self._last_coord).any(): | ||
| # self._delta_stream.append(np.linalg.norm(current_coord - self._last_coord)) | ||
| # self._last_coord = current_coord | ||
| # except: | ||
| # pass | ||
| # finally: | ||
| # self._curve.setData(self._delta_stream) | ||
|
|
||
| # try: | ||
| # self._x_stream.append(filtered_vision_data.yellow_robots[0].x) | ||
| # self._y_stream.append(filtered_vision_data.yellow_robots[0].y) | ||
| # except: | ||
| # pass | ||
| # finally: |
Copilot
AI
Dec 9, 2025
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.
This comment appears to contain commented-out code.
| # # for b_robot in combined_vision_data.blue_robots: | |
| # # writer.writerow({ | |
| # # TS_COL: combined_vision_data.ts, | |
| # # ID_COL: b_robot.id, | |
| # # COLOR_COL: "blue", | |
| # # X_COL: b_robot.x, | |
| # # Y_COL: b_robot.y, | |
| # # TH_COL: b_robot.orientation | |
| # # }) | |
| # self.data_collected += 1 | |
| # For live testing: | |
| # try: | |
| # current_coord = np.array(( | |
| # filtered_vision_data.yellow_robots[2].x, | |
| # filtered_vision_data.yellow_robots[2].y | |
| # )) | |
| # if not np.isnan(self._last_coord).any(): | |
| # self._delta_stream.append(np.linalg.norm(current_coord - self._last_coord)) | |
| # self._last_coord = current_coord | |
| # except: | |
| # pass | |
| # finally: | |
| # self._curve.setData(self._delta_stream) | |
| # try: | |
| # self._x_stream.append(filtered_vision_data.yellow_robots[0].x) | |
| # self._y_stream.append(filtered_vision_data.yellow_robots[0].y) | |
| # except: | |
| # pass | |
| # finally: | |
| # pass | |
| # finally: |
| # if not np.isnan(self._last_coord).any(): | ||
| # self._delta_stream.append(np.linalg.norm(current_coord - self._last_coord)) |
Copilot
AI
Dec 9, 2025
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.
This comment appears to contain commented-out code.
| # except: | ||
| # pass | ||
| # finally: | ||
| # self._curve.setData(self._x_stream, self._y_stream) |
Copilot
AI
Dec 9, 2025
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.
This comment appears to contain commented-out code.
| # self._curve.setData(self._x_stream, self._y_stream) |
|
|
||
| def ball_merge_predicate(b1: RawBallData, b2: RawBallData) -> bool: | ||
| return abs(b1.x - b2.x) + abs(b1.y - b2.y) < CameraCombiner.BALL_MERGE_THRESHOLD | ||
|
|
Copilot
AI
Dec 9, 2025
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.
Normal methods should have 'self', rather than 'b1', as their first parameter.
| def ball_merge_predicate(b1: RawBallData, b2: RawBallData) -> bool: | |
| return abs(b1.x - b2.x) + abs(b1.y - b2.y) < CameraCombiner.BALL_MERGE_THRESHOLD | |
| @staticmethod | |
| def ball_merge_predicate(b1: RawBallData, b2: RawBallData) -> bool: | |
| return abs(b1.x - b2.x) + abs(b1.y - b2.y) < CameraCombiner.BALL_MERGE_THRESHOLD | |
| @staticmethod |
|
|
||
| def ball_merge_predicate(b1: RawBallData, b2: RawBallData) -> bool: | ||
| return abs(b1.x - b2.x) + abs(b1.y - b2.y) < CameraCombiner.BALL_MERGE_THRESHOLD | ||
|
|
Copilot
AI
Dec 9, 2025
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.
Normal methods should have 'self', rather than 'b1', as their first parameter.
| def ball_merge_predicate(b1: RawBallData, b2: RawBallData) -> bool: | |
| return abs(b1.x - b2.x) + abs(b1.y - b2.y) < CameraCombiner.BALL_MERGE_THRESHOLD | |
| @staticmethod | |
| def ball_merge_predicate(b1: RawBallData, b2: RawBallData) -> bool: | |
| return abs(b1.x - b2.x) + abs(b1.y - b2.y) < CameraCombiner.BALL_MERGE_THRESHOLD | |
| @staticmethod |
| from collections import defaultdict, deque | ||
| # import csv | ||
| from dataclasses import replace | ||
| import matplotlib.pyplot as plt |
Copilot
AI
Dec 9, 2025
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.
Import of 'plt' is not used.
| import matplotlib.pyplot as plt |
| from dataclasses import replace | ||
| import matplotlib.pyplot as plt | ||
| import numpy as np | ||
| import pyqtgraph as pg # type: ignore |
Copilot
AI
Dec 9, 2025
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.
Import of 'pg' is not used.
| import pyqtgraph as pg # type: ignore |
| import matplotlib.pyplot as plt | ||
| import numpy as np | ||
| import pyqtgraph as pg # type: ignore | ||
| from pyqtgraph.Qt import QtWidgets # type: ignore |
Copilot
AI
Dec 9, 2025
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.
Import of 'QtWidgets' is not used.
| from pyqtgraph.Qt import QtWidgets # type: ignore |
Position refiners noise filtering
Completed by @szeyoong-low and @AndrewAha