implement multi-farm support in pywake_api - #62
Conversation
|
Hi @kilojoules, I did a quick agent-based review. It found a few things. High
Medium
Low
|
|
is this only for pywake? can we setup a new example case, such that I can also adopt foxes to read multiple farms from windio? Thanks |
Address PR #62 review: - dedupe turbines across farms by full spec, not name; raise on same-name/different-spec conflicts - key hub_heights by global type index so specs with distinct hub heights no longer collide on per-farm "0" keys - take FUGA rotor diameter from merged specs instead of farms[0] - per-turbine AEP on same normalization basis as total, so per-farm AEP sums match the returned total - drop wall-clock assertion from multifarm test; assert outputs exist - fold duplicated create_turbines/_build_farm_turbine_list into create_turbine + _build_multifarm_turbines Add examples/cases/multiple_wind_farms from the IEA Wind 2200-22-MW Reference Offshore Wind Plant (Apache-2.0): three farms sharing one site and resource, for other tool APIs (foxes) to adopt. Multi-farm wind_farm arrays validate only against upstream IEAWindSystems/windIO schemas, so point the pin there; move AWAKEN cutin/cutout under performance to conform. Refs #61
|
Pushed 6ec490f addressing the review findings and adding the requested example case:
|
|
@bjarketol I think my coding agent adressed these things, detailed in the above message. Does this look OK to commit? |
|
@SchmJo I added a multi wind farm example |
|
@kilojoules Supported by foxes now. Added to foxes tests, and they are green. |
|
Second-round agent review (4 agents: fix verification, fresh bug hunt, data/schema/cross-tool, test adequacy), at 952e8cd. Round 1: all 8 findings verified fixed in 6ec490f — including numerically confirming per-farm AEPs sum to the total on both Weibull and time-series paths, and that New critical findings
Medium
Low
Verified goodType indexing matches Recommendation: hold merge until criticals 1–4 are addressed. Highest-value tests: multi-type single farm with 1-based keys, |
- turbine_types layout entries resolved as mapping keys (1-based or arbitrary keys work), not positional indices - single-spec turbine_types without per-position layout types falls back to that type instead of raising KeyError (regression from main) - explicit ValueError when layout turbine_types length differs from the coordinate arrays (was silent per-farm AEP misattribution) - turbine spec dedup comparison tolerates numpy arrays - time-series multi-hub-height path: build the site when turbulence_intensity is absent (was site=None crash), dedupe heights correctly when types share a hub height, average per-turbine TI - wayve, floris, and code_saturne raise NotImplementedError with a clear message on multi-farm input instead of a raw TypeError - test_foxes_multiple_farms skips when foxes < 1.8.4 cannot read a wind_farm list - pin windIO and foxes git dependencies to the locked commits - docs: point windIO install instructions at upstream IEAWindSystems - ROWP example: README with Apache-2.0 attribution, consistent USD/m cost comments - run_pywake docstring documents the list[float] multi-farm return - tests: per-farm AEPs sum to the merged-farm total (Weibull and time-series), 1-based turbine_types keys example, dict-form layouts, length-mismatch error, single-spec fallback, same-hub-height types, multi-height without TI, floris/wayve NotImplementedError guards
uv.lock is gitignored, so CI resolves floating git dependencies fresh on every run. lejeunemax/floris commit 9a59ffa (2026-07-31) remapped Bastankhah2014 to floris_name=None, which made floris raise "Wind Deficit Model 'Bastankhah2014' is not implemented in FLORIS" and broke the four floris example tests on CI. Pin floris to 4f367c39, the last commit where the windIO reader maps Bastankhah2014 to gauss, and pin wayve to the same commit uv.lock resolves, matching the windIO and foxes pins.
- _specs_equal: NaN-aware, fully recursive over lists/tuples/arrays; identical specs with NaN placeholders merge, nested list-of-dict specs with numpy values no longer raise ambiguous-truth ValueError - layout turbine_types: entries that match the turbine_types mapping keys resolve as keys; entries that don't but are valid 0-based integers fall back to positional indices (windIO schema wording) with a warning; anything else raises with both interpretations explained - time-series site construction is dims-aware throughout: variables are time-subset and turbine-averaged along their declared dims instead of hard-coded axes, so ['wind_turbine','time'] ordering works - per-turbine TI on the multi-hub-height path warns loudly that the farm-mean is used; TI is height-interpolated only when it declares a height dim (was an uncaught IndexError with height-coordinate wind data) - default operating array sized to the times_run subset length, not the full mask length - drop now-unused _interpolate_with_min - foxes multi-farm test asserts 100 turbines and positive power instead of only running - docs: pin windIO in the manual install command to the pyproject commit - ROWP example README documents the upstream EUR/USD label inconsistency - tests: positional-index fallback, NaN spec merge, _specs_equal edge cases, height-coordinate + per-turbine TI, times_run subset, and turbine-first dims regressions
- turbine_types resolution: exact key matches always win and string coercion never shadows a distinct literal key; positional fallback only applies when NO entry matches a key (mixed resolution now raises instead of silently flipping matched entries); booleans rejected as positional indices - _specs_equal: 0-d numpy arrays compare as scalars instead of raising 'iteration over a 0-d array' - height handling unified in _interp_along_height: interpolation keyed on each variable's declared height dim with extrapolate + 0.02 TI floor in both single- and multi-type branches; declaring a height dim without a resource height coordinate raises a clear ValueError; 2-D data with no declared dims infers a trailing height axis when the resource declares heights (restores pre-refactor behavior); replaces the axis-guessing _interpolate_wind_data try/except heuristic, which could silently interpolate along the wrong axis when the subset length equaled the height count - turbine averaging deduplicated into _mean_over_turbines (TI + both density sites) - foxes multi-farm test uses tmp_path (no leaked output dir on failure) - tests: key-resolution rules, 0-d array specs, undeclared-dims 2-D TI, height-dim-without-coordinate error, height-first/time-first equivalence under the poison-case subset, hub height outside the resource height range
- turbine_types resolution finalized: full-key resolution wins; otherwise valid 0-based integer entries resolve positionally with a warning that spells out the per-entry type assignment (restores main-branch support for 1-based keys used with 0-based indices); string-key coercion works again for integer entries (only skipped when it would shadow a different literal key); YAML booleans are rejected outright instead of aliasing integer keys 0/1 through dict hashing - undeclared-dims inference validates axis lengths against the height coordinate and turbine count: unambiguous shapes are labeled, anything ambiguous raises asking for explicit dims (no more silent height/turbine axis confusion) - height-resolved density is interpolated to hub height via the shared helper instead of crashing XRSite with 2-D data; density extraction deduplicated into get_density_series - extrapolation floor applies only outside the declared height range: in-range TI below 0.02 is preserved; wind speed floors at 0 when extrapolated - wind direction interpolates along height via sin/cos components, so 350/10 degrees no longer averages to 180 - tests: resolution-rule matrix (1-based keys, string-key coercion, boolean rejection, unresolvable entries), floor/wraparound unit tests, height-resolved density, ambiguous undeclared dims
|
Checked cc1dd75 locally. All second-round findings are resolved — criticals 1–5 verified by direct probes (not only the new tests), mediums/lows addressed too. 68 tests pass in both the default env and Only blocker: [Claude code on behalf of Bjarke] |
3 participants
No description provided.