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] Allow empty arrays in export as dict #1061

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

rafmudaf
Copy link
Collaborator

@rafmudaf rafmudaf commented Feb 6, 2025

Bug fix: Allow empty arrays in export as dict

In #1040, functionality was added to support initializing the FlorisModel from a default configuration. I intentionally left the wind speed, wind direction, and turbulence intensity inputs as empty arrays instead of some default numeric value so that the FlorisModel.run() function would fail with an error requiring the user to set these inputs themselves. However, if FLORIS is initialized and some parameters set but not the wind speed, wind direction, and TI, then these arrays are dropped during the FlorisModel.set() routine.

Details:

  1. FlorisModel.set() calls FlorisModel._reinitialize()
  2. FlorisModel._reinitialize() exports FlorisModel.Core to a dictionary with as_dict from the FromDictMixin class
    • Note Core inherits from BaseClass which inherits from FromDictMixin
  3. FromDictMixin.as_dict() calls attr.asdict with the _attr_floris_filter filter
  4. _attr_floris_filter filters out the following conditions from the export:
    • The attribute is not initialized when the object is created (field(init=False)
    • The attribute's value is None
    • The length is 0 when the attribute is a np.array
  5. The last condition above removes arrays that are set to empty during the default init
  6. FlorisModel._reinitialize() calls Core.from_dict(), but the empty arrays have been removed
  7. FlowField requires wind speed, wind direction and turbulence intensity inputs, so there's an error

This pull request removes the last condition in _attr_floris_filter. While the complete impact of this is difficult to assess, all the tests pass and the examples run without change. Also, I added an error within the last condition to check if it's used at all in the examples, and it was not called (see Test Results).

Related issue

No open issue, but introduced in #1040 and spotted in WISDEM/Ard#47

Test results, if applicable

Before removing it, I added an error in the _attr_floris_filter condition to check whether any examples execute this branch.

@rafmudaf rafmudaf requested a review from misi9170 February 6, 2025 21:26
@rafmudaf rafmudaf self-assigned this Feb 6, 2025
Copy link
Collaborator

@misi9170 misi9170 left a comment

Choose a reason for hiding this comment

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

Thanks @rafmudaf . Once we get all of the green ticks I'll merge.

@misi9170 misi9170 changed the title Allow empty arrays in export as dict [BUGFIX] Allow empty arrays in export as dict Feb 6, 2025
@misi9170 misi9170 merged commit f48e897 into NREL:develop Feb 6, 2025
11 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.

2 participants