-
Notifications
You must be signed in to change notification settings - Fork 10
Reconcile cory's and main offshore-dev #105
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
Reconcile cory's and main offshore-dev #105
Conversation
* liberate python 3.13 * actually liberate python * update testing * version number inequality fix * walkback python for backdated wisdem * propogate to tests
…shore-development
…shore-development
…shore-development
…shore-development
…shore-development
…shore-development
…shore-development
…shore-development
…re generic latent variable set up
* (prep to) liberate python 3.13 (NLRWindSystems#69) * liberate python 3.13 * actually liberate python * update testing * version number inequality fix * walkback python for backdated wisdem * propogate to tests * remove comments
* (prep to) liberate python 3.13 (NLRWindSystems#69) * liberate python 3.13 * actually liberate python * update testing * version number inequality fix * walkback python for backdated wisdem * propogate to tests * remove comments * constraint testing added * black reformatting * added updated pyrite * system test added but not complete for mooring packing * add forgotten file * Black reformat * switch to using OM derivative checks, clean up * black reformat * Update test/system/ard/geometry/test_constraints.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update test/system/ard/geometry/test_constraints.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update test/system/ard/offshore/test_mooring_packing.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update test/system/ard/offshore/test_mooring_packing.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update test/system/ard/geometry/test_constraints.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * tidy comments * fix the problem discovered by the not problem that jared pointed out hahahaha * barely avoided ragequitting due to black reformat --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* (prep to) liberate python 3.13 (NLRWindSystems#69) * liberate python 3.13 * actually liberate python * update testing * version number inequality fix * walkback python for backdated wisdem * propogate to tests * remove comments * update pyrite values * fixes to 3.21 rather than 3.20 * update test utils for clarity * a little more improvement to the test utils * black reformat...
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.
Looks fine to me
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 merges the latest changes from the main develop branch into offshore-dev, bringing test updates, dependency bumps, and core refactors into sync.
- Realign and streamline unit and system tests to match updated utility and BOS APIs
- Refactor BOS cost components to integrate
SpacingApproximationsand centralize latent variable setup - Update dependencies, CI workflows, and example scripts to reflect new APIs and version constraints
Reviewed Changes
Copilot reviewed 20 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/utils/test_utils.py | Removed unused imports and left placeholder test class |
| test/unit/utils/test_matematics.py | Adjusted JAX imports for grad and test_util |
| test/unit/ard/cost/test_wisdem_wrap.py | Swapped LandBOSSE for LandBOSSEArdComp in unit tests |
| test/system/ard/offshore/test_mooring_packing.py | Added full system test for mooring packing |
| test/system/ard/geometry/test_constraints.py | Introduced boundary constraint system tests |
| test/system/ard/cost/test_spacing_approximations_connections.py | Updated import path for spacing approximation group |
| pyproject.toml | Restricted Python and dependency version ranges |
| examples/onshore/optimization_demo.py | Updated BOS import and turbine spec path |
| examples/offshore/optimization_demo.py | Replaced hardcoded if True with optimize flag |
| examples/offshore-fixed/optimization_demo.py | Corrected Orbit import to ORBIT |
| ard/utils/test_utils.py | Renamed validator parameter and added traceback for failures |
| ard/layout/sunflower.py | Documented spacing_target input and switched to FD partials |
| ard/layout/gridfarm.py | Switched complex-step to finite-difference for partials |
| ard/glue/prototype.py | Replaced LandBOSSE with LandBOSSEArdComp in the glue layer |
| ard/cost/wisdem_wrap.py | Major refactor: introduced SpacingApproximations and helper API |
| ard/cost/approximate_turbine_spacing.py | Removed duplicate BOS group, left only SpacingApproximations |
| README.md | Relaxed WISDEM install command |
| .github/workflows/python-tests-unit.yml | Restricted Python versions and removed dev-install step |
| .github/workflows/python-tests-system.yml | Restricted Python versions and removed dev-install step |
| .coveragerc | Updated omitted paths for coverage |
Comments suppressed due to low confidence (3)
.github/workflows/python-tests-unit.yml:33
- Removing the
pip install .[dev]step means dev dependencies (like pytest) may not be installed, causing the CI tests to fail. Re-add the dev install or explicitly install required test dependencies.
pip install .[dev]
.github/workflows/python-tests-system.yml:34
- The removal of the dev dependencies installation step will likely break system tests due to missing test frameworks. Please restore or explicitly install the test requirements.
pip install .[dev]
test/unit/utils/test_utils.py:1
- [nitpick] This test file no longer contains any actual test methods after trimming imports; consider adding tests for
pyrite_validatorto maintain coverage for utility functions.
class TestUtils:
| for full_name in promotion_map: | ||
| prom_name = promotion_map[full_name] | ||
| core_name = prom_name.split(".")[-1] | ||
| if core_name in promotion_map: |
Copilot
AI
Jul 14, 2025
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.
This checks for core_name in promotion_map but should be verifying membership in variable_map before calling prob.set_val to avoid unintended lookups.
| if core_name in promotion_map: | |
| if core_name in variable_map: |
| try: | ||
| prob.set_val(prom_name, variable_map[core_name]) | ||
| except: | ||
| print( |
Copilot
AI
Jul 14, 2025
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.
[nitpick] Using print for warnings may go unnoticed; consider using Python’s logging module or OpenMDAO’s reporting to surface missing inputs more robustly.
| print( | |
| logging.warning( |
cb9cb3a
into
NLRWindSystems:offshore-development
This is really just bringing in develop