Bugfix/optiwindnet jitter overlapping #163
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merge develop changes into main
Update installation instructions
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request addresses a critical bug where optiwindnet crashes (sometimes with segfaults) when turbines or substations have overlapping coordinates. The fix introduces jitter when overlaps are detected to allow simulations to run successfully.
Key Changes
- Adds comprehensive test coverage for the viewshed calculation functionality
- Implements tests for optiwindnet overlap handling with jitter and warning verification
- Introduces multi-objective optimization test infrastructure
- Adds new logging utilities for component-level stdout/stderr capture
- Updates configuration files to use newer YAML structure (
objectivesinstead ofobjective) - Fixes documentation typos
Reviewed changes
Copilot reviewed 26 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/ard/layout/test_viewshed.py | New comprehensive test suite for viewshed area calculations with overlapping and non-overlapping turbine configurations |
| test/unit/ard/collection/test_optiwindnet.py | Adds tests for handling coincident turbines/substations with jitter and warning verification |
| test/unit/ard/api/test_multiobjective.py | New test file for multi-objective optimization setup and validation |
| test/unit/ard/api/test_interface_unit.py | Minor formatting improvement to error message |
| ard/utils/logging.py | New utility module for component-level logging with stdout/stderr capture |
| Multiple YAML files | Configuration updates to use objectives dict instead of objective structure |
| examples/data/windIO-plant_wind-resource_wrg-example.yaml | Removes 0.0 wind speed bin from probability data |
| docs/installation.md | Fixes shell command typos |
| .gitignore | Adds ard_prob_out directory |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jaredthomas68
requested changes
Dec 17, 2025
Contributor
jaredthomas68
left a comment
jaredthomas68
left a comment
There was a problem hiding this comment.
Looks pretty good. Main comment is that when I read "jitter" I though you meant "just-in-time compiler". Can we change the terminology to "translation", "shift", or something similar? I think this is particularly important given our use of jax and possibly later julia where jit is a common term, and does not mean "a little shake".
jaredthomas68
approved these changes
Dec 17, 2025
Contributor
jaredthomas68
left a comment
jaredthomas68
left a comment
There was a problem hiding this comment.
One super minor comment. Looks good!
cfrontin
added a commit
that referenced
this pull request
Feb 6, 2026
* fix typos in installation instructions (#152) * Multi-objective refactor (#153) * Bump version from 0.1.0-beta0 to 0.1.0-beta1 * added first cut at multi-objective stuff * Bump version from 0.1.0-beta1 to 0.1.0-beta2 * multi-objective refactor * fix example code * black reformat * invert erroneous exclusion * improved unit testing * get rid of pyoptsparse dependencies and broaden test coverage * fixed erroneous inclusion of options * address copilot comments * address jared's requests on #153 * add the file that i renamed the wrong way --------- Co-authored-by: Jared Thomas <jaredthomas68@users.noreply.github.com> * Add a simple viewshed component (#156) * added viewshed and empty test component. * Bump version from 0.1.0-beta0 to 0.1.0-beta1 * Bump version from 0.1.0-beta1 to 0.1.0-beta2 * viewshed should be done * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * restore change i didn't like --------- Co-authored-by: Jared Thomas <jaredthomas68@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove zero velocity resource for FLORIS (#160) * Bump version from 0.1.0-beta0 to 0.1.0-beta1 * Bump version from 0.1.0-beta1 to 0.1.0-beta2 * removed zero velocity rows from floris results * adjust values with lt one percent error due to renormalization after wind resource pdf adjustment --------- Co-authored-by: Jared Thomas <jaredthomas68@users.noreply.github.com> * Create a new automated stdout/stderr redirection system (#161) * Bump version from 0.1.0-beta0 to 0.1.0-beta1 * Bump version from 0.1.0-beta1 to 0.1.0-beta2 * prototype stdout/stderr capturing implemented. * missed a bunch, trying again. * fix bounds manager * adjust file paths * clear ipynb * remove viz script which isn't actually involved in that pr * Apply typo suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fixed black formatting * wip logging with iterations * black reformatting * remove messy iteration debug statements * black reformat again * update optimization demo * update logging code for error handling and docs * made stdio capture optional modeling option and updated * bring floris input file generator in with logger * added case name to address @jaredthomas68's comment on #161 * cleanup some iter_count discovery and defaulting code w/ multiple returns * updated the logging docstrings * ard system fixed * fixing stuff and added some features related to the fixes * try again to overcome windows error * add teardown to fix windows file access issue * use case_name distinction to make sure the reports end up in unique places * wip try teardown to fix file access on windows * fix discovered typo, test mistake * added another necessary teardown * forgot two more * retry * black reformat... * adjust case names for better use * black reformat... * ... stupid omission --------- Co-authored-by: Jared Thomas <jaredthomas68@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Bugfix/optiwindnet jitter overlapping (#163) * Bump version from 0.1.0-beta0 to 0.1.0-beta1 * Bump version from 0.1.0-beta1 to 0.1.0-beta2 * updated tests to check jitter * address comments from copilot * add substation test * deal with jared's comments * black reformat and adding comments jared asked for * added convenience printing to test. * final black reformatting --------- Co-authored-by: Jared Thomas <jaredthomas68@users.noreply.github.com> * WISDEM NSGA2 incorporation (#158) * Bump version from 0.1.0-beta0 to 0.1.0-beta1 * Bump version from 0.1.0-beta1 to 0.1.0-beta2 * added NSGA2 to optimizer options * removed zero velocity rows from floris results * add example 06 * udpate * adjust pyproject to use dev branch * added gfortran for running on GH action runner * undo dev branching * black reformat plus added copilot-requested changes for viz utils * added forgotten file * add unit testing for nsga2 implementation * address @jaredthomas68 comments * add ard yaml --------- Co-authored-by: Jared Thomas <jaredthomas68@users.noreply.github.com> * Feature/goodybag (#165) * readme adjustments to address and close #154 and numpy version adjustment to close #157 * refactor the test organization for better incorporation of subpackages * black reformat * update github runner * address jared suggestions * fix logomaker (#166) * fix logomaker * black reformat * WISDEM update and configurations (#172) * Bump version from 0.1.0-beta0 to 0.1.0-beta1 * Bump version from 0.1.0-beta1 to 0.1.0-beta2 * update for WISDEM/ORBIT pyrite standard value changes * fix typo --------- Co-authored-by: Jared Thomas <jaredthomas68@users.noreply.github.com> * Add domain exclusion zones via windIO (#167) * bring in the exclusions and their viz implications * added exclusion code * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * new gold standard test, weird failure * fixed value of the exclusion distance; still failing now on two: mysterious sign flip, sometimes... * add a boundary test case that lights up the strange error * rename and add some tests * added exclusion test even though it's in tatters * debugging * use switch instead of pad to fix boundary in/out identification. also clean up debug statements * black reformat * black reformat --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jared Thomas <jaredthomas68@gmail.com> * Black reformat update to 2026 stable format (#173) * Bump version from 0.1.0-beta0 to 0.1.0-beta1 * Bump version from 0.1.0-beta1 to 0.1.0-beta2 * update for WISDEM/ORBIT pyrite standard value changes * black v2026 reformat --------- Co-authored-by: Jared Thomas <jaredthomas68@users.noreply.github.com> * Eagle density function for eco-constrained design setups (#168) * bring in the exclusions and their viz implications * added exclusion code * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * added eagle density code and tested it * fix filepath * improved documentation, added and tested derivatives on eagles splines * black reformat * first tranche of copilot changes * second tranche of copilot fixes * Apply suggestions from copilot code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * more consistent error messages * black reformat * new gold standard test, weird failure * fixed value of the exclusion distance; still failing now on two: mysterious sign flip, sometimes... * add a boundary test case that lights up the strange error * rename and add some tests * added exclusion test even though it's in tatters * debugging * use switch instead of pad to fix boundary in/out identification. also clean up debug statements * black reformat * black reformat * address jared comments * fix accidentally stashed and dropped changes * black reformat... * fix calling convention * black are you kidding me --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jared Thomas <jaredthomas68@gmail.com> * update org name (#170) Co-authored-by: Cory Frontin <cory.frontin@nrel.gov> * Add some helpers for "house style" plots (#174) * create house style update * update house style to actually work * Release/develop (#177) * Bump version from 0.1.0-beta0 to 0.1.0-beta1 * Bump version from 0.1.0-beta1 to 0.1.0-beta2 * eliminating wrongthink just kidding removed references to WISDEM repo and replaced with renamed NLRWindSystems --------- Co-authored-by: Cory Frontin <cory.frontin@nrel.gov> * Develop backmerge (#178) * Bump version from 0.1.0-beta0 to 0.1.0-beta1 * Bump version from 0.1.0-beta1 to 0.1.0-beta2 * eliminating wrongthink just kidding removed references to WISDEM repo and replaced with renamed NLRWindSystems --------- Co-authored-by: Jared Thomas <jaredthomas68@users.noreply.github.com> * Update ard/utils/geometry.py Remove pad_polygon * Update test/ard/unit/utils/test_geometry.py Remove tests for no longer used `pad_polygon` function * Update docs/testing.md Remove duplicate line Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * correct connection x->y is now x->x * run black * Update ard/viz/house_style.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * correct turbine connections and remove unused gplot import * run black --------- Co-authored-by: Jared Thomas <jaredthomas68@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jared Thomas <jaredthomas68@gmail.com> Co-authored-by: Pietro Bortolotti <ptrbortolotti@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I figured out that optiwindnet will crash, sometimes with segfaults, if turbines or substations overlap. This inserts a jitter iff there's an overlap on those coordinates to allow simulations to run through.