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

Various changes to code and documentation #613

Merged
merged 12 commits into from Apr 25, 2023
Merged

Conversation

rafmudaf
Copy link
Collaborator

Collection of random changes and improvements

This pull request includes a variety of changes and improvements that don't easily fall under other ongoing work. The changes are listed below.

  • 25e380d - Replace hard-coded rotor diameter with model value
  • 2a30913 - Add all releases to quality metric plots
  • dfc00c2 - Remove redundant array slice operations
  • 362d97c - Disable GCH components for Jensen example input
  • afd7dfc - Simplify time-series selection in FlorisInterface
  • bc6a281 - Remove unused turbine fCps array
  • a02e68a - Use dict over array for turbine property map (see Farm-class attribute type #594)
  • 9dc507f - Remove redundant math in wind direction delta
  • a888e41 - Fix and improve a few docstrings
  • 9b856f7 - git-ignore Ruff cache files
  • 27b5ca6 - Add info about GitHub form comments

These changes only impact the quality of the code itself. There are no API or functionality changes.

Related issue

No issue, but there are some comments:

Impacted areas of the software

The code within FlorisInterface, the solvers, Farm, FlowField, and utilities.wind_delta are modified, but the functionality and results in all of these modules should not change.

Test results, if applicable

Here's a listing of where the changes are tested.

  • 9dc507f - Remove redundant math in wind direction delta
    • utilities_unit_test already has a test for this
  • a02e68a - Use dict over array for turbine property map (see Farm-class attribute type #594)
    • tested in turbine_unit_test
  • bc6a281 - Remove unused turbine fCps array
    • tested in regression tests; the results are unchanged and the code runs so this is a valid change
  • afd7dfc - Simplify time-series selection in FlorisInterface
    • See the attached output below from the time series example (21) before and after this change
  • dfc00c2 - Remove redundant array slice operations
    • See below
  • 25e380d - Replace hard-coded rotor diameter with model value
    • TODO: Cumulative-Curl regression tests value are unchanged, but this should be tested with a wind farm with turbines of different rotor diameters; @bayc do we have such a test?

Time series switch

These plots are the results from the time series example (21). The first is before the change to the time series flag and the second is after the change. They are different because that example creates a random-walk for the wind directions. However, this show that the time series flag is enabled correctly, and this verifies the change.

Redundant array slices

This is verified with the same example as above, 21. The plot below shows that the code runs, as expected, so removing the extra slices did not impact the shape of the arrays.

@rafmudaf rafmudaf added the enhancement An improvement of an existing feature label Mar 29, 2023
@rafmudaf rafmudaf added this to the v3.4 milestone Mar 29, 2023
@rafmudaf rafmudaf requested a review from bayc March 29, 2023 21:39
@rafmudaf rafmudaf self-assigned this Mar 29, 2023
@rafmudaf
Copy link
Collaborator Author

rafmudaf commented Apr 6, 2023

@bayc the specific change I'm hoping to get your eyes on are:

  • b4af594 - Replace hard-coded rotor diameter with model value
    • Is rotor_diameter_i correct?
  • bd626f6 - Remove redundant array slice operations
    • Sanity check
  • a84d24e - Remove unused turbine fCps array
    • Sanity check
  • 38655da - Use dict over array for turbine property map (see Farm-class attribute type #594)
    • Sanity check
  • 1ab7952 - Remove redundant math in wind direction delta
    • Sanity check

@bayc
Copy link
Collaborator

bayc commented Apr 25, 2023

@rafmudaf Thanks for calling these out specifically! Responses are in italics below.

  • b4af594 - Replace hard-coded rotor diameter with model value
    • Is rotor_diameter_i correct? Yes, that is correct.
  • bd626f6 - Remove redundant array slice operations
    • Sanity check. That checks out on my end.
  • a84d24e - Remove unused turbine fCps array
    • Sanity check. I had added this one because I've needed to get Cp values in the past for certain analyses/customers at each turbine, but didn't add a method to floris_interface to get turbine Cps. So I think this would mainly be useful for that.
  • 38655da - Use dict over array for turbine property map (see Farm-class attribute type #594)
    • Sanity check. Good catch! Seems right to me.
  • 1ab7952 - Remove redundant math in wind direction delta
    • Sanity check. Also looks good!

@rafmudaf
Copy link
Collaborator Author

Thanks for the review @bayc. Is it worth retaining the fCp arrays then?

@rafmudaf rafmudaf merged commit b4e538f into NREL:develop Apr 25, 2023
3 checks passed
@rafmudaf rafmudaf deleted the random branch April 25, 2023 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants