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

minor clean-ups to floris.py #419

Merged
merged 6 commits into from
Jan 16, 2025
Merged

Conversation

elenya-grant
Copy link
Collaborator

@elenya-grant elenya-grant commented Jan 13, 2025

Floris wrapper clean-up

Previously when using floris to simulate the wind farm, it would write the wind resource data to speed_dir_data.csv. This file is never read or used anywhere else in the code - so this has been removed. All other relevant information for this PR is found in the below section.

Note: this also includes a patch fix for building readthedocs (see more information here)

PR Checklist

  • RELEASE.md has been updated to describe the changes made in this PR
  • Documentation
    • Docstrings are up-to-date
    • Related docs/ files are up-to-date, or added when necessary
    • Documentation has been rebuilt successfully
    • [-] Examples have been updated
  • Tests pass (If not, and this is expected, please elaborate in the tests section)
  • PR description thoroughly describes the new feature, bug fix, etc.

Related issues

Impacted areas of the software

  • hopp/simulation/technologies/wind/floris.py
    • value(): fixed the set_value functionality to work as documented
    • execute(): cleaned up how operational losses are applied and to only print 'Simulating wind farm output in FLORIS...' if verbose is True (verbose is a new attribute which defaults to True)
    • parse_resource_data: removed part that exports wind resource data to speed_dir_data.csv (this was unused and does not impact the functionality at all)

Additional supporting information

Test results, if applicable

@elenya-grant elenya-grant marked this pull request as ready for review January 13, 2025 19:59
Copy link
Collaborator

@RHammond2 RHammond2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good to me from a software perspective!

Copy link
Collaborator

@genevievestarke genevievestarke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Good to merge!

@johnjasa johnjasa merged commit 21ea863 into NREL:develop Jan 16, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants