-
Notifications
You must be signed in to change notification settings - Fork 9
Add domain exclusion zones via windIO #167
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: develop
Are you sure you want to change the base?
Conversation
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 support for domain exclusion zones in wind farm layouts through the windIO configuration system. The implementation mirrors the existing boundary distance handling and provides a new component for calculating distances between turbines and exclusion zones.
Key Changes:
- New
FarmExclusionDistancePolygonOpenMDAO component for computing turbine-to-exclusion-zone distances - Comprehensive test suite for single and multi-polygon exclusion zones including derivative validation
- Visualization support for rendering exclusion zones in layout plots
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
ard/layout/exclusions.py |
New OpenMDAO component implementing polygon exclusion zone distance calculations with automatic differentiation via JAX |
test/ard/unit/layout/test_exclusion.py |
Comprehensive test suite validating exclusion distance computation and derivatives for single and multiple polygon scenarios |
test/ard/unit/layout/test_boundary.py |
Cleanup of unused imports and variables that were likely used during development |
ard/viz/layout.py |
Extended plotting functionality to render exclusion zones as semi-transparent red polygons |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
jaredthomas68
left a comment
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.
Minor comments, not really blocking. I did not math check your tests, but I assume you did.
| "exclusions": { | ||
| "polygons": [ | ||
| { | ||
| "x": [0.0, 1000.0, 1000.0, 0.0], |
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 would prefer to see this test use a case closer to the expected use case. In this test, all turbines are inside the exclusion zone. Could you make the exclusion zone smaller, with maybe just one turbine inside and at least one turbine right on the boundary and other turbines outside?
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.
all these tests, not just this one
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.
tests updated in 44c1b37. failing one in a way I can't explain.
Adds exclusion zones that mirror boundaries using the windIO setup.