Initial scaffolding build for geophysical research#222
Conversation
There was a problem hiding this comment.
Pull request overview
This PR establishes an automated pipeline for geophysical navigation research by creating a comprehensive set of simulation configurations and analysis tools. It replaces the previous manual configuration approach with a structured pipeline that systematically tests UKF and EKF filters with different combinations of geophysical measurements (gravity and magnetic anomalies).
Changes:
- Adds 6 new simulation configuration files for systematic testing of geophysical navigation under various sensor configurations (gravity-only, magnetic-only, and combined measurements for both UKF and EKF filters)
- Replaces interactive progress indicators with proper debug logging in the simulation core
- Implements automated pipeline tasks in pixi.toml for running simulations and post-processing results
- Adds geophysical performance analysis capability to compare geophysical-aided navigation against degraded baseline navigation
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test_geo_config_wizard.sh | Removed obsolete test script for configuration wizard |
| pixi.toml | Added hardcoded sim and postprocess tasks for automated 8-configuration pipeline execution |
| pixi.lock | Updated package hash for analysis module |
| core/src/sim.rs | Replaced stdout print statements with debug! logging for better library usage |
| conf/ukf_truth.toml | Added logging configuration section |
| conf/ukf_mag.toml | New configuration for UKF with magnetic anomaly measurements only |
| conf/ukf_grav.toml | New configuration for UKF with gravity anomaly measurements only |
| conf/ukf_degraded.toml | Added log file configuration (with placement issue) |
| conf/ukf_both.toml | New configuration for UKF with both gravity and magnetic measurements |
| conf/geonav_example.toml | Removed example configuration, replaced by new systematic configs |
| conf/ekf_truth.toml | Added logging configuration section |
| conf/ekf_mag.toml | New configuration for EKF with magnetic anomaly measurements only |
| conf/ekf_grav.toml | New configuration for EKF with gravity anomaly measurements only |
| conf/ekf_degraded.toml | Added logging configuration section |
| conf/ekf_both.toml | New configuration for EKF with both gravity and magnetic measurements |
| analysis/src/analysis/plotting.py | Added plot_relative_performance function for geophysical performance visualization |
| analysis/src/analysis/init.py | Added geoperformance command and analysis function for comparative analysis |
| .gitignore | Changed from specific PNG ignore to blanket data/output/* ignore |
Comments suppressed due to low confidence (3)
analysis/src/analysis/plotting.py:285
- 'except' clause does nothing but pass and there is no explanatory comment.
except Exception:
analysis/src/analysis/plotting.py:375
- 'except' clause does nothing but pass and there is no explanatory comment.
except Exception:
analysis/src/analysis/plotting.py:389
- 'except' clause does nothing but pass and there is no explanatory comment.
except Exception:
|
|
||
| [closed_loop] | ||
| filter = "ukf" | ||
| file = "log/ukf_mag.log" |
There was a problem hiding this comment.
The log file is configured in both the [logging] section (line 10) and the [closed_loop] section (line 14). This duplication is inconsistent with other config files and could cause confusion or parsing errors. The file field should only be specified in the [logging] section and should be removed from the [closed_loop] section.
| file = "log/ukf_mag.log" |
|
|
||
|
|
There was a problem hiding this comment.
Trailing whitespace on this line. This should be removed to maintain code cleanliness and consistency.
| deg: DataFrame, # degraded/baseline navigation | ||
| nav: DataFrame, # GPS-aided truth |
There was a problem hiding this comment.
The inline parameter comments (lines 155-156) are not following proper Python docstring conventions. These should be documented in the docstring using a standard format (e.g., Args section with proper parameter descriptions) rather than inline comments. This would improve consistency with the rest of the codebase and provide better IDE support for documentation.
| references = list(Path(args.reference).glob("*.csv")) | ||
| print(f"Found {len(references)} reference datasets.") | ||
|
|
||
| print(f"Comparing to degraded data in: {args.degraded}") | ||
| degradeds = list(Path(args.degraded).glob("*.csv")) | ||
| print(f"Found {len(degradeds)} degraded datasets.") |
There was a problem hiding this comment.
The variables references and degradeds are computed but never used. These lines can be removed as the actual file paths are constructed later using reference_path / dataset.name and degraded_path / dataset.name.
| references = list(Path(args.reference).glob("*.csv")) | |
| print(f"Found {len(references)} reference datasets.") | |
| print(f"Comparing to degraded data in: {args.degraded}") | |
| degradeds = list(Path(args.degraded).glob("*.csv")) | |
| print(f"Found {len(degradeds)} degraded datasets.") | |
| print(f"Comparing to degraded data in: {args.degraded}") |
| print(f"Degraded file for {dataset.name} not found in {degraded_path}. Skipping.") | ||
| continue | ||
| output_plot = output_path / f"{dataset.stem}_geophysical_performance.png" | ||
| print(f"Processing dataset {dataset} ({len(nav)}) with reference {reference_file.name} ({len(nav)}) and degraded {degraded_file.name} ({len(degraded_nav)})") |
There was a problem hiding this comment.
The print statement incorrectly uses len(nav) twice. The first occurrence should be len(geo) to correctly report the length of the geo dataset being processed.
| print(f"Processing dataset {dataset} ({len(nav)}) with reference {reference_file.name} ({len(nav)}) and degraded {degraded_file.name} ({len(degraded_nav)})") | |
| print(f"Processing dataset {dataset} ({len(geo)}) with reference {reference_file.name} ({len(nav)}) and degraded {degraded_file.name} ({len(degraded_nav)})") |
|
|
||
| [geophysical] | ||
| # Magnetic anomaly measurements | ||
| magnetic_resolution = "one_minute" |
There was a problem hiding this comment.
Inconsistent magnetic resolution configuration: ukf_mag.toml uses "one_minute" while ukf_both.toml, ekf_mag.toml, and ekf_both.toml use "two_minutes". If this is intentional for experimental comparison, it should be documented in a comment. Otherwise, consider using consistent values (all "one_minute" or all "two_minutes") for fair comparison across filter configurations.
| magnetic_resolution = "one_minute" | |
| magnetic_resolution = "two_minutes" |
| @@ -6,6 +6,7 @@ generate_plot = true | |||
|
|
|||
| [closed_loop] | |||
| filter = "ukf" | |||
There was a problem hiding this comment.
The log file configuration is inconsistently placed. In this file, file = "log/ukf_degraded.log" is under the [closed_loop] section (line 9), but in other config files like ukf_truth.toml, ukf_mag.toml, etc., it's correctly placed under the [logging] section. This inconsistency could lead to configuration parsing errors or unexpected behavior. The file field should be moved from the [closed_loop] section to the [logging] section.
| filter = "ukf" | |
| filter = "ukf" | |
| [logging] |
| distance_traveled = haversine_vector( | ||
| nav[["latitude", "longitude"]].to_numpy()[:-1, :], | ||
| nav[["latitude", "longitude"]].to_numpy()[1:, :], | ||
| Unit.METERS, | ||
| ) | ||
| distance_traveled = np.hstack(([0], distance_traveled)) | ||
| distance_traveled = np.nancumsum(distance_traveled) | ||
|
|
There was a problem hiding this comment.
Variable distance_traveled is not used.
| distance_traveled = haversine_vector( | |
| nav[["latitude", "longitude"]].to_numpy()[:-1, :], | |
| nav[["latitude", "longitude"]].to_numpy()[1:, :], | |
| Unit.METERS, | |
| ) | |
| distance_traveled = np.hstack(([0], distance_traveled)) | |
| distance_traveled = np.nancumsum(distance_traveled) |
PR submits a re-do of the updated underlying simulation and new UKF/EKF filters with semi-automated performance analysis pipeline generation. Additional tuning and a fine-grained anaylsis is still required.