-
Notifications
You must be signed in to change notification settings - Fork 155
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
Require TI must be length n_findex in core code #831
Conversation
hi @rafmudaf @misi9170 and @ejsimley , I think this is now just about ready for review, I kept the checklist above up to date as I went so should provide a good overview of what changed and why. I originally thought it smart to update examples in this pull request but now think it's maybe best to separate that piece out, so crossed it out. If you could have a look would appreciate! |
Ah shoot no, I need to update all the examples, sorry, one last thing |
Ok I think all examples are fixed and this is ready for review |
There was a problem hiding this 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, aside form some clarification in the docstrings about the heterogenous_inflow_config dimensions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made a few last docstring updates. I think it looks good now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't pulled down and run, but this looks good overall to me
Mostly around the tab level for arguments that are split by line
Thanks for all the feedback gang, merging now! |
Require TI must be n_findex in core code
After some discussions we agreed that a consistent to handle a few issues would be for the "core" code to require the turbulence intensities must be n_findex. This avoids ambiguities and cases where the code might perform unexpectedly. However, to simplify things for the user, the wind_data objects, WindRose, WindTIRose and TimeSeries will each now include methods for broadcasting a provided float value to match the given dimensions.
Further, the approach proposed in PR #821, where heterogeneity can be described with respect to wd, in addition along findex, will be adapted in this pull request to be methods of the respective WindData objects. This enables that FlorisInterface can go back to requiring het_map to be have 0th dimension n_findex long, simplifying that code.
Steps in this draft then:
Update the first example around these new rulesRelated issue
PR #821