-
Notifications
You must be signed in to change notification settings - Fork 10
Glue draft #94
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
Glue draft #94
Conversation
* liberate python 3.13 * actually liberate python * update testing * version number inequality fix * walkback python for backdated wisdem * propogate to tests
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.
is this right? an empty yaml file?
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.
empty file?
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.
empty file?
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 looks great!
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.
empty file?
| - ["AEP_farm", "financese.plant_aep_in"] | ||
| - ["landbosse.total_capex_kW", "financese.bos_per_kW"] | ||
| systems: | ||
| layout2aep: |
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.
approximate subtotals?
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.
Done
| import ard.glue.prototype as glue | ||
|
|
||
| # import ard.api.prototype as glue | ||
| import ard.api.interface as glue2 |
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.
is this permanent?
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.
Fixed
| # # plot convergence | ||
| # ## read cases | ||
| # cr = om.CaseReader( | ||
| # prob.get_outputs_dir() | ||
| # / input_dict["analysis_options"]["recorder"]["filepath"] | ||
| # ) | ||
| # | ||
| # # Extract the driver cases | ||
| # cases = cr.get_cases("driver") | ||
| # | ||
| # # Initialize lists to store iteration data | ||
| # iterations = [] | ||
| # objective_values = [] | ||
| # | ||
| # # Loop through the cases and extract iteration number and objective value | ||
| # for i, case in enumerate(cases): | ||
| # iterations.append(i) | ||
| # objective_values.append( | ||
| # case.get_objectives()[ | ||
| # input_dict["analysis_options"]["objective"]["name"] | ||
| # ] | ||
| # ) | ||
| # | ||
| # # Plot the convergence | ||
| # plt.figure(figsize=(8, 6)) | ||
| # plt.plot(iterations, objective_values, marker="o", label="Objective (LCOE)") | ||
| # plt.xlabel("Iteration") | ||
| # plt.ylabel("Objective Value (Total Cable Length (m))") | ||
| # plt.title("Convergence Plot") | ||
| # plt.legend() | ||
| # plt.grid() | ||
| # plt.show() | ||
| # | ||
| # optiwindnet.plotting.gplot(prob.model.optiwindnet_coll.graph) | ||
| # | ||
| # plt.show() |
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 have this in the analysis script now, so we can probably remove it from here, I think?
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 do you think @jaredthomas68
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 am inclined to leave it. Since this give users trying the examples some outputs to work with. We should consider creating some better viz tools so we don't need so much code though.
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.
Actually, I think we should just put the rest of optimization_demo.py into the Jupyter notebook and blow away the .py file. It is confusing to have to run. multiple files for a single example. I'd rather have everything in Jupyter unless there is a run time concern or something
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 can be a new PR though since this PR is already pretty bloated.
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.
oh yeah, that's a good idea. maybe all of these examples should be the same way? let's commit as-is for now and create a new PR to clean the examples to all use notebooks
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
cfrontin
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.
Let's get this merged conditioned on a new PR to fix up the examples for a consistent notebook format.
|
I brought in the optiwindnet changes. I don't think they should break anything, but eyeball the changes I made to rectify those! |
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 updates the test suite to align with the new glue/API code, expanding and standardizing how modeling options are passed into tests across multiple modules. Key changes include:
- Augmenting
modeling_optionsin many tests to explicitly set layout and orientation parameters (spacing_primary,spacing_secondary,angle_orientation,angle_skew,x_turbines,y_turbines,x_substations,y_substations). - Migrating from legacy
ard.glue.prototypefunctions to the newset_up_ard_modelAPI and removing obsolete imports. - Refactoring test setup to reduce duplication via helper functions (
make_modeling_options) and adopting subtests in pyrite file validations.
Reviewed Changes
Copilot reviewed 80 out of 90 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/utils/test_io.py | Added nested-list/dict tests for replace_key_value. |
| test/unit/ard/layout/test_spacing.py | Removed unused JAX imports. |
| test/unit/ard/layout/test_gridfarm.py | Extended modeling_options with spacing and angle keys. |
| test/unit/ard/farm_aero/test_templates.py | Refactored wind query instantiation and integrated into modeling_options. |
| test/unit/ard/farm_aero/test_floris.py | Updated pyrite validation to use subtests; now imports pytest. |
| test/unit/ard/cost/test_wisdem_wrap.py | Switched to load-only pyrite validation and added subtests. |
| test/unit/ard/collection/test_templates.py | Added coordinate arrays for turbines and substations. |
| test/unit/ard/collection/test_optiwindnet.py | Introduced make_modeling_options, restructured OpenMDAO setup. |
| test/unit/ard/api/test_interface_unit.py | New unit tests for set_up_ard_model error handling. |
| test/system/ard/* | Updated system tests to use set_up_ard_model, include new params, and supply input files under inputs_*. |
Comments suppressed due to low confidence (3)
test/unit/ard/farm_aero/test_floris.py:231
- Missing import for
Pathfrompathlib. Addfrom pathlib import Pathat the top of the file so thatPath(...)resolves correctly.
Path(__file__).parent / "test_floris_aep_pyrite.npz",
test/unit/ard/cost/test_wisdem_wrap.py:171
- Missing import for
numpy as npat the top of the file. Addimport numpy as npso thatnp.iscloseis available.
assert np.isclose(value, pyrite_data[key], rtol=5e-3), (
test/unit/ard/cost/test_wisdem_wrap.py:8
- Missing import for
Pathfrompathlib. Addfrom pathlib import Pathso that file-path operations work as intended.
import ard.utils.test_utils
|
i'm coming in super late here, but should we add a |
I think this is for later. I created an issue for it #111 |
The core glue code seems to work. However, the way the wisdom wrapper was set up was incompatible with the glue code. This was an initial attempt to get working.