Add math engine, report panel, and CSV export (Milestone 4)#2
Conversation
Implement the core features of Milestone 4: computed math channels, statistical reporting, and data export. Math Engine (i3rs-core): - Recursive descent expression parser with operator precedence - Evaluator with 20+ built-in functions (smooth, derivative, integrate, abs, trig, pow, clamp, etc.) - Automatic frequency resampling when mixing channels of different rates - Flexible channel name resolution (underscore/space/dot, case-insensitive) - CSV export with multi-frequency resampling UI Integration (i3rs-app): - ChannelId enum (Physical/Math) replacing raw usize throughout - Math editor panel: add/edit/delete/evaluate expressions with error display - Math channels in channel browser with drag-and-drop support - Math channels plottable on graphs identically to physical channels - Report panel: min/max/avg/stddev per channel per lap - Math file save/load (JSON) and workspace persistence - CSV export of plotted channels via File menu Shared utilities: - compute_channel_stats() in state.rs (unified from duplicate impls) - MathChannelDef::new() constructor - Extracted draw_legend_entry() helper from near-duplicate methods - build_channel_data_map() only decodes referenced channels
![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 implements Milestone 4, introducing a math expression engine, a statistics report panel, and CSV export functionality. Key changes include a hand-written recursive descent parser and evaluator for math expressions, a math channel editor UI, and a new ChannelId abstraction to unify physical and math channels across the application. Feedback highlights critical performance concerns in the report panel due to per-frame statistics recalculation, the need for dependency resolution when evaluating math channels, and a suggestion to use the trapezoidal rule for more accurate numerical integration.
…names - Cache report panel statistics in ReportCache, only recompute when displayed channels or laps change (fixes per-frame O(N) recomputation) - Evaluate math channels in topological dependency order using Kahns algorithm so inter-channel references resolve correctly - Reject duplicate math channel names (case-insensitive check against both physical and existing math channels) - Switch cumulative_integral to trapezoidal rule for better accuracy
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements Milestone 4, introducing a math expression engine, a math channel editor, and a statistics report panel. Key features include a recursive descent parser, an evaluator with built-in functions and automatic resampling, and CSV export capabilities. The UI has been refactored to support both physical and math channels via a unified ChannelId. Review feedback focuses on performance optimizations, specifically addressing O(L^2) complexity in the report panel, redundant decoding of physical channels, and excessive memory allocations caused by unnecessary data cloning during resampling and evaluation.
- Use O(1) selected lap check instead of O(L) position() search in report - Remove redundant in_degree initialization in topological sort - Use Cow in resample() to avoid clone when frequencies already match
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements Milestone 4, introducing a comprehensive math engine and reporting system. Key additions include a recursive descent expression parser, an evaluator with built-in DSP functions (smoothing, derivatives, integrals), a math channel editor UI, and a statistics report panel. The architecture now utilizes a unified ChannelId to handle both physical and computed channels across the graph and browser panels. Feedback focuses on performance optimizations for large telemetry datasets, specifically addressing unnecessary data cloning during evaluation, the lack of scalar broadcasting for constants, and
1 participant
Summary
i3rs-core: recursive descent parser, evaluator with 20+ built-in functions (smooth,derivative,integrate, trig, etc.), automatic frequency resampling across channels, flexible name resolutionChannelIdabstractionTest plan
cargo build --release— clean build, zero warningscargo test— 53 tests passing (35 core + 14 integration + 4 CLI)derivative(Speed)), verify it appears in browser and plots correctly