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

Enhancement: Add a Dimension Validator for 5-D and 3-D Array Structures #763

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

RHammond2
Copy link
Collaborator

Validate the Dimensions of 5-D and 3-D Arrays

This PR adds new validation functionality for arrays that should adhere to dimensions requirements of N wind directions x N wind speeds x N turbines or N wind directions x N wind speeds x N turbines x N grid x N grid. An additional mixin class that wraps attrs.validate(class_instance) has been added to easily enable attrs class validators to be run at key points in a simulation.

Related issue

At least partially resolves #761 by using the factory=lambda: np.array([]) paradigm to not build extensive carveouts for data that should only be valid at initialization.

Impacted areas of the software

floris.type_dec.py

  • validate_5DArray_shape is an attrs validator checking the adherence to the shape being N wind directions x N wind speeds x N turbines x N grid x N grid, with the exception of empty arrays and those that can be broadcast along the grid axes.
  • validate_3DArray_shape is an attrs validator checking the adherence to the shape being N wind directions x N wind speeds x N turbines, with the exception of empty arrays and those that can be broadcast along the wind speed axis.
  • validate_mixed_dim runs the 5D, then 3D validation to check for the turbulence_intensity attributes that start as 5D, then are reduced to 3D.
  • array_3D_field is a custom field for defining a field with the 3D validator and lambda factory built in to reduce the number of multi-line attribute initializations.
  • array_5D_field is a custom field for defining a field with the 5D validator and lambda factory built into reduce the number of multi-line attribute initializations.
  • array_mixed_dim_field is a custom field for defining a field with the 3D/5D validator and lambda factory built into reduce the number of multi-line attribute initializations.
  • ValidateMixin provides self.validate(), which is a wrapper for attrs.validate(class_instance) to easily run all validators in a class.

floris/simulation/base.py

  • BaseClass: now inherits ValidateMixin

floris/simulation/farm.py and floris/simulation/flow_field.py

  • Adopts the array_3D_field, array_5D_field, and array_mixed_dim_field as appropriate

floris/simulation/grid.py

  • Fixes some typing errors.
  • Adopts the array_3D_field and array_5D_field where appropriate

floris/tests/floris_unit_test.py

  • Fixes a bug in creating a simulation object with incorrect wind speed and wind direction dimensions

floris/tests/type_dec_unit_test.py

  • Adds in a test for ValidateMixin.validate() and the new dimensions validators.

Additional supporting information

This largely stems from a conversation with @rafmudaf about questions of how to check if attributes are staying the same shape throughout a simulation, and adding methods to ensure the correctness of dimension sizes.

Test results, if applicable

Tests are passing with a slight modification to an existing test that was using incorrect dimensions.

@RHammond2 RHammond2 added bug Something isn't working enhancement An improvement of an existing feature labels Dec 13, 2023
@rafmudaf rafmudaf self-assigned this Dec 13, 2023
@rafmudaf rafmudaf added this to the v3.6 milestone Dec 13, 2023
Copy link
Collaborator

@paulf81 paulf81 left a comment

Choose a reason for hiding this comment

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

hi @RHammond2, I reviewed this code and all looks good to me! I went through the changes and re-ran the tests locally. Do I understand right the plan is to confirm this all works in 5D, then merge into 4D work and update there?

@RHammond2
Copy link
Collaborator Author

RHammond2 commented Dec 13, 2023

Thanks, @paulf81, and that's the plan for now, or if the 4D work happens first, this branch will get updated prior to merging. The idea was to keep the work separated and avoiding dependent PRs on either end.

@paulf81
Copy link
Collaborator

paulf81 commented Dec 14, 2023

Thanks, @paulf81, and that's the plan for now, or if the 4D work happens first, this branch will get updated prior to merging. The idea was to keep the work separated and avoiding dependent PRs on either end.

That sounds good!

@RHammond2
Copy link
Collaborator Author

@rafmudaf the examples all work on my machine now, so this should be ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement An improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants