Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds new functionality for computing aerodynamic stability derivatives and introduces an angle-of-attack (AOA) correction flag, along with several example scripts and supporting data files for the TUDELFT_V3_KITE configuration.
Key Changes:
- Added functions to compute aircraft-frame aerodynamic coefficients and build polynomial coefficient tables for stability derivative analysis
- Introduced
is_aoa_correctedparameter to decouple AOA correction logic from aerodynamic model type selection - Added multiple example scripts demonstrating geometry plotting, literature comparison, benchmark testing, and AOA correction validation
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/VSM/stability_derivatives.py | Added three new functions for computing aircraft-frame coefficients from solver, fitting quadratic polynomials, and building coefficient tables; includes duplicate imports that should be removed |
| src/VSM/plotting.py | Added logic to compute L/D ratio for polar plots, but has a critical bug where it unconditionally overwrites steering-specific data |
| src/VSM/core/Solver.py | Added is_aoa_corrected boolean parameter to control AOA correction behavior; missing parameter documentation |
| src/VSM/core/BodyAerodynamics.py | Refactored AOA correction logic to use is_aoa_corrected flag instead of checking aerodynamic_model_type |
| examples/TUDELFT_V3_KITE/tutorial.py | Modified file path assignments with duplicate/commented code that should be cleaned up |
| examples/TUDELFT_V3_KITE/plotting_the_geometry.py | New example script demonstrating geometry visualization using matplotlib and plotly |
| examples/TUDELFT_V3_KITE/fit_polynomials_polars.py | Updated file paths and removed print statement; contains typo in commented code |
| examples/TUDELFT_V3_KITE/compare_against_literature.py | New script for comparing VSM results against CFD and wind tunnel data |
| examples/TUDELFT_V3_KITE/benchmark_runtime.py | New script for benchmarking solver performance with different initialization strategies |
| examples/TUDELFT_V3_KITE/aoa_correction_test.py | New script for testing and comparing AOA correction effects |
| data/TUDELFT_V3_KITE/CAD_derived_geometry/struc_geometry_surfplan.yaml | Updated bridle connection headers and indices |
| data/TUDELFT_V3_KITE/3D_polars_literature/*.csv | Added new wind tunnel and CFD reference data files for validation |
Comments suppressed due to low confidence (14)
src/VSM/core/BodyAerodynamics.py:972
- This comment appears to contain commented-out code.
# else:
# side_prescribed_va = jit_dot(
# lift_induced_va, spanwise_direction
# ) + jit_dot(drag_induced_va, spanwise_direction)
src/VSM/plotting.py:592
- Variable handle_list is not used.
handle_list = []
src/VSM/core/Solver.py:135
- This assignment to 'alpha_array' is unnecessary as it is redefined before this value is used.
This assignment to 'alpha_array' is unnecessary as it is redefined before this value is used.
This assignment to 'alpha_array' is unnecessary as it is redefined before this value is used.
This assignment to 'alpha_array' is unnecessary as it is redefined before this value is used.
This assignment to 'alpha_array' is unnecessary as it is redefined before this value is used.
alpha_array = np.zeros(self.n_panels)
src/VSM/core/BodyAerodynamics.py:3
- Import of 'pd' is not used.
import pandas as pd
src/VSM/core/BodyAerodynamics.py:4
- Import of 'Path' is not used.
from pathlib import Path
src/VSM/plotting.py:4
- Import of 'tempfile' is not used.
import tempfile
src/VSM/plotting.py:5
- Import of 'subprocess' is not used.
import subprocess
src/VSM/plotting.py:11
- Import of 'Axes3D' is not used.
from mpl_toolkits.mplot3d import Axes3D
src/VSM/plotting.py:12
- Import of 'Poly3DCollection' is not used.
from mpl_toolkits.mplot3d.art3d import Poly3DCollection
src/VSM/plotting.py:14
- Import of 'get_monitors' is not used.
from screeninfo import get_monitors
src/VSM/plotting.py:15
- Import of 'Line2D' is not used.
from matplotlib.lines import Line2D
src/VSM/plotting.py:16
- Import of 'plot_on_ax' is not used.
from VSM.plot_styling import set_plot_style, plot_on_ax
src/VSM/core/BodyAerodynamics.py:974
- This assignment assigns a variable to itself.
side_prescribed_va = side_prescribed_va
src/VSM/plotting.py:644
- This statement is unreachable.
raise ValueError(
"is_show and is_save are both True. Please set one of them to False."
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| q: float = 0.0, | ||
| r: float = 0.0, | ||
| reference_point: Optional[np.ndarray] = None, | ||
| ) -> Tuple[Dict[str, float], float, float]: |
There was a problem hiding this comment.
Missing docstring for function compute_aircraft_frame_coeffs_from_solver. This function appears to be a public API function that performs coordinate transformations and solver calls. It should have comprehensive documentation explaining:
- The purpose of the function
- Parameter descriptions (especially the units for
alpha_rad,beta_rad, etc.) - Return value description (what each element of the tuple represents)
- The coordinate frame transformations being applied
| ) -> Tuple[Dict[str, float], float, float]: | |
| ) -> Tuple[Dict[str, float], float, float]: | |
| """ | |
| Compute aerodynamic force and moment coefficients in the aircraft frame by | |
| invoking the VSM solver with specified flight conditions and body rates. | |
| This function maps aircraft-frame inputs (angles, rates) to the VSM solver's | |
| conventions, runs the solver, and transforms the resulting force and moment | |
| coefficients back to the aircraft frame. | |
| Parameters | |
| ---------- | |
| body_aero : object | |
| The aerodynamic model object compatible with the VSM solver. | |
| solver : object | |
| The VSM solver instance, which must provide a `solve` method and may | |
| optionally have a `reference_point` attribute and `rho` (density). | |
| alpha_rad : float | |
| Angle of attack in radians (aircraft frame, positive = nose up). | |
| beta_rad : float | |
| Sideslip angle in radians (aircraft frame, positive = nose right). | |
| V : float | |
| True airspeed magnitude in meters per second (m/s). | |
| p : float, optional | |
| Roll rate in radians per second (rad/s), aircraft frame (default: 0.0). | |
| q : float, optional | |
| Pitch rate in radians per second (rad/s), aircraft frame (default: 0.0). | |
| r : float, optional | |
| Yaw rate in radians per second (rad/s), aircraft frame (default: 0.0). | |
| reference_point : np.ndarray, optional | |
| Reference point for moment calculation (meters, shape (3,)). If None, | |
| uses solver's reference_point or [0, 0, 0]. | |
| Returns | |
| ------- | |
| coeffs_air : dict | |
| Dictionary of nondimensional force and moment coefficients in the | |
| aircraft frame, with keys: | |
| - "CX": X-force coefficient (positive rearward) | |
| - "CY": Y-force coefficient (positive right) | |
| - "CZ": Z-force coefficient (positive downward) | |
| - "Cl": Roll moment coefficient (positive right wing down) | |
| - "Cm": Pitch moment coefficient (positive nose up) | |
| - "Cn": Yaw moment coefficient (positive nose right) | |
| q_inf : float | |
| Dynamic pressure in Pascals (Pa). | |
| S : float | |
| Reference area in square meters (m^2). | |
| Notes | |
| ----- | |
| - Input angles and rates are mapped from the aircraft frame to the VSM | |
| solver's conventions: | |
| * alpha (deg): aircraft +ve = nose up, VSM +ve = nose up | |
| * beta (deg): aircraft +ve = nose right, VSM +ve = nose left (hence sign flip) | |
| * p, r: sign flip to match VSM axes | |
| - Output coefficients are mapped back to the aircraft frame using | |
| R_y(pi) = diag(-1, +1, -1) for forces and moments. | |
| """ |
|
|
||
|
|
||
| # --- Main builder ------------------------------------------------------------------ | ||
| def build_malz_coeff_table_from_solver( |
There was a problem hiding this comment.
The function name build_malz_coeff_table_from_solver uses "malz" which appears to be an unclear or non-standard abbreviation. Consider using a more descriptive name like build_coefficient_table_from_solver or build_polynomial_coefficient_table_from_solver if "malz" refers to a specific methodology that should be documented.
| def build_malz_coeff_table_from_solver( | |
| def build_coefficient_table_from_solver( |
| if angle_type == "angle_of_attack": | ||
| polar_data[3] = df["CL"].values / df["CD"].values |
There was a problem hiding this comment.
This unconditionally overwrites polar_data[3] with CL/CD whenever angle_type == "angle_of_attack", regardless of the steering parameter. This logic conflicts with lines 559-562 which conditionally set polar_data[3] based on the steering parameter.
The condition on line 563 should either:
- Be moved inside an
elseblock after the steering conditions (lines 558-562), or - Include additional checks to avoid overwriting steering-specific data
This appears to be a logic error that would cause incorrect plots when angle_type == "angle_of_attack" and steering is set to "sideforce" or "roll".
| if angle_type == "angle_of_attack": | |
| polar_data[3] = df["CL"].values / df["CD"].values | |
| elif angle_type == "angle_of_attack": | |
| polar_data[3] = df["CL"].values / df["CD"].values |
| artificial_damping: dict = {"k2": 0.1, "k4": 0.0}, | ||
| is_with_simonet_artificial_viscosity: bool = False, | ||
| simonet_artificial_viscosity_fva: float = None, | ||
| is_aoa_corrected: bool = False, |
There was a problem hiding this comment.
Missing documentation for the new is_aoa_corrected parameter. The docstring for __init__ (lines 55-76) does not include documentation for this new parameter. Please add it to maintain consistency with other parameters.
| # file_path=(cad_derived_geometry_dir / "config_kite_CAD_CFD_polars.yaml"), | ||
| file_path=(cad_derived_geometry_dir / "config_kite_CAD_CFD_polars.yaml"), | ||
| spanwise_panel_distribution=spanwise_panel_distribution, | ||
| bridle_path=( |
There was a problem hiding this comment.
Duplicate file path assignment. Line 67 is commented out but line 68 assigns the same file path. This appears to be leftover debugging/development code. The commented line 67 should be removed, or if a different path was intended, it should be uncommented and line 68 should be removed.
| # file_path=(cad_derived_geometry_dir / "config_kite_CAD_CFD_polars.yaml"), | |
| file_path=(cad_derived_geometry_dir / "config_kite_CAD_CFD_polars.yaml"), | |
| spanwise_panel_distribution=spanwise_panel_distribution, | |
| bridle_path=( | |
| file_path=(cad_derived_geometry_dir / "config_kite_CAD_CFD_polars.yaml"), | |
| spanwise_panel_distribution=spanwise_panel_distribution, | |
| bridle_path=( | |
| bridle_path=( |
| from VSM.plotting import ( | ||
| plot_polars, | ||
| ) | ||
| from VSM.plot_geometry_matplotlib import plot_geometry |
There was a problem hiding this comment.
Import of 'plot_geometry' is not used.
| from VSM.plot_geometry_matplotlib import plot_geometry |
| plot_polars, | ||
| ) | ||
| from VSM.plot_geometry_matplotlib import plot_geometry | ||
| from VSM.plot_geometry_plotly import interactive_plot |
There was a problem hiding this comment.
Import of 'interactive_plot' is not used.
| from VSM.plot_geometry_plotly import interactive_plot |
| from VSM.plotting import ( | ||
| plot_polars, | ||
| ) | ||
| from VSM.plot_geometry_matplotlib import plot_geometry |
There was a problem hiding this comment.
Import of 'plot_geometry' is not used.
| from VSM.plot_geometry_matplotlib import plot_geometry |
| plot_polars, | ||
| ) | ||
| from VSM.plot_geometry_matplotlib import plot_geometry | ||
| from VSM.plot_geometry_plotly import interactive_plot |
There was a problem hiding this comment.
Import of 'interactive_plot' is not used.
| from VSM.plot_geometry_plotly import interactive_plot |
| from VSM.core.BodyAerodynamics import BodyAerodynamics | ||
| from VSM.core.Solver import Solver | ||
| from VSM.plotting import ( | ||
| plot_polars, |
There was a problem hiding this comment.
Import of 'plot_polars' is not used.
| plot_polars, |
No description provided.