-
Notifications
You must be signed in to change notification settings - Fork 159
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
Support initializing FLORIS with default values #1040
Conversation
69a6dd9
to
309f535
Compare
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.
hi @rafmudaf this looks good to me. I checked the code changes and ran the tests. Thanks for adding additional documentation. Finally, just wanted to note I agree with GCH as default since that is also default in examples.
TODO Add an option to print some or all default values |
Allows for using this file in other areas of the software
ce9bc9b
to
2c7d975
Compare
@misi9170 I've added a function here to display the current configuration in a short and long form. See https://github.com/rafmudaf/floris/blob/defaults_init/docs/advanced_concepts.ipynb for an example. |
…er interface changes.
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.
This looks great, thanks @rafmudaf . I made a small change to re-add the print_dict
method (with a deprecation warning) as a wrapper to show_config
in case anyone was using it, I don't want to introduce a breaking change.
Assuming you are happy with that, I can get this merged in.
FYI, this is super helpful for me! I've been copying the default input yamls between different repositories. |
Good catch @misi9170 and I appreciate you making that change! I'm happy with it. @Bartdoekemeijer once you integrate this into your workflow, I'd love to hear any feedback. I'll also be using it soon, so I'm sure we'll refine the API over the next few months. |
The defaults initialization feature was completed in NREL/floris#1040
Add an option to initialize FLORIS with default options for use as a library
When used as a library in third-party software, the calling-code typically must carry along a version of the FLORIS input files or require them as user input in order to initialize FLORIS. Often, the inputs are immediately changed by the calling-code such as in an optimization routine. This pattern leads to unnecessary complexity when integrating FLORIS in third-party software, and it could be avoided by providing an option to initialize FLORIS with default data with the understanding that much of it will be overwritten. See #1037.
This pull request modifies the initialization method of
FlorisModel
to load some set of default values whenconfiguration="defaults"
. Additionally, a method to retrieve the default inputs as a Python dictionary is provided so that it can be retrieved and modified prior to initialization from the calling code. This is required, for example, if the calling code will change a value that isn't supported inFlorisModel.set(...)
such as the wake model parameters. I've also added a documentation block in the Advanced Concepts section.Design considerations
Another input file
I considered setting defaults through the
attrs.field
function, but decided against this since it would spread the set of default parameters over many source code files. Instead, they're all contained in one file,floris/default_inputs.py
. It hurts my heart to add yet another input file to the FLORIS repository, and I apologize in advance to any future developer who has to update these 💔.What are good defaults?
I chose the GCH example input file as the defaults. Is that reasonable?
Guessable
Is this confusing to users who use FLORIS directly rather than through another tool? If so, how can it be made more explicit?