Skip to content
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

[BUGFIX] Coordinates getter on Farm class #846

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

misi9170
Copy link
Collaborator

Addresses issue raised in #833 that getter Farm.coordinates fails when hub_heights has been expanded to dimensions (n_wind_directions x n_wind_speeds x n_turbines) (from its original shape of n_turbines).

After considering the two possible fixes mentioned in #833, I've decided to go with the latter, because it affects less of the code. When investigating the former option (which removes the broadcasting of hub_heights to be 3-dimensional in the first place), I found that I could remove the broadcast of both hub_heights and rotor_diameters and still pass tests, but removing the broadcast of turbine_type_map causes failures. For consistency and minimal invasiveness, I've gone with a higher-level patch here.

@misi9170 misi9170 changed the base branch from main to develop March 18, 2024 19:20
@misi9170 misi9170 changed the title Fix coordinates getter on Farm class [BUGFIX] Coordinates getter on Farm class Mar 18, 2024
@misi9170 misi9170 added the bug Something isn't working label Mar 18, 2024
@misi9170
Copy link
Collaborator Author

Note that a similar bugfix will be needed on the v4 branch, although this may be resolved by merging develop back into v4.

@rafmudaf rafmudaf merged commit 4b33139 into NREL:develop Mar 19, 2024
8 checks passed
@misi9170 misi9170 deleted the bugfix/iss833 branch March 20, 2024 15:52
@misi9170 misi9170 mentioned this pull request Apr 5, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants