-
Notifications
You must be signed in to change notification settings - Fork 0
Add a geometry from yaml constructor for Wing #186
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?
Add a geometry from yaml constructor for Wing #186
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…r V3 Kite, and adjusting plotting script to handle arbitrary column sorting of literature input
…indTunnel Data and Python
@jellepoland when you feel like this pull request is ready, you can convert it from a draft pull request to a normal pull request by marking it ready for review, and request a review from me and/or uwe. |
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.
Very cool stuff, thanks for the contribution! 🚀 Left some comments, please address these.
data/TUDELFT_V3_KITE/wing_geometry_CAD_CFD_polars_pchip_fitted.yaml
Outdated
Show resolved
Hide resolved
Great comments! My main takeaway is the struct--constructor interaction in Julia, that I am not used to in Python.The defining of a single struct, and multiple functions with the same name as that struct (e.g. Wing) all being able to take in different input types, and Julia then automatically picking the appropriate functions. |
On which branch did you address my comments? I don't see it here... |
I am still working on the changes! I am changing and extending the |
Reorganized the testing suite and added some tests. |
Getting a lot of these warnings in the tests, please fix these before we merge this branch @jellepoland
|
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.
Please, fix the failing tests on windows and the "method overwritten" warnings first.
Furthermore, add a description that explains what you changed.
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 adds YAML-based wing geometry construction capabilities to VortexStepMethod.jl, enabling users to define wings through configuration files with embedded polar data references.
Key changes:
- Implement
yaml_geometry.jl
module with Wing constructors that read YAML configuration files - Add support for loading aerodynamic polar data from CSV files referenced in YAML
- Reorganize test suite into module-specific directories with comprehensive test coverage
Reviewed Changes
Copilot reviewed 73 out of 111 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/yaml_geometry.jl | Core YAML geometry parsing with Wing constructors and polar data loading |
test/yaml_geometry/ | Comprehensive test suite with sample YAML/CSV files for wing geometry |
test/test_data_utils.jl | Shared utilities for test data management across modules |
src/VortexStepMethod.jl | Export new YAML geometry functionality |
examples/ | Example scripts demonstrating YAML-based wing construction |
data/ | Sample configuration files for pyramid and V3 kite models |
Summary of Key Changes from This Branch1. Core New Functionality: YAML Geometry Support
2. Enhanced Settings System
3. Comprehensive Test Infrastructure
4. Data and Examples
5. Core Module Improvements
|
Windows test passing |
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.
I think this can be merged now, @jellepoland can you update NEWS.md?
I updated the News.md |
…xstepmethodjl-and-run-on-a-simple-wing' of github.com:OpenSourceAWE/VortexStepMethod.jl into 183-create-a-load_geometry_from_yaml-function-for-vortexstepmethodjl-and-run-on-a-simple-wing
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.
While I approve it, please still find the time to answer my questions/ address my comments.
@@ -0,0 +1,9 @@ | |||
alpha,beta,CL,CD,CS,CMy |
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.
What is CMy ?
project_dir = dirname(dirname(pathof(VortexStepMethod))) # Go up one level from src to project root | ||
|
||
# Load VSM vsm_settings from YAML configuration file | ||
vsm_settings = VSMSettings("pyramid_model/vsm_settings.yaml") |
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.
What is a pyramid model?
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.
It is a model that has the shape of a pyramid :) You can run the example and it will plot it.
end | ||
|
||
""" | ||
load_polar_data(csv_file_path::String) -> Tuple{Union{Nothing, Tuple{Vector{Float64}, Vector{Float64}, Vector{Float64}, Vector{Float64}}}, Symbol} |
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.
If you return such a complicated structure, isn't it better to define a struct type for it and just document that you return a PolarData struct or how ever you call it?
No description provided.