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

Rename floris.simulation, floris.tools to floris.core, floris #830

Merged
merged 46 commits into from
Mar 12, 2024

Conversation

rafmudaf
Copy link
Collaborator

@rafmudaf rafmudaf commented Mar 5, 2024

Rename FLORIS packages

This pull request restructures the packages within FLORIS.

v3 Architecture

Currently, the software has the following high-level architecture:

classDiagram
    class tools {
        +FlorisInterface
    }
    class simulation {
        +Floris
    }
    class type_dec
    class utilities
    tools <-- type_dec
    simulation <-- type_dec
    tools <-- utilities
    simulation <-- utilities
    tools <-- simulation
Loading

The floris.tools.FlorisInterface class is the entry point for most use-cases, and it uses floris.simulation as a library for computing the wake calculation with the Floris class as the central data structure. This architecture has been around since at least v2, but it has some issues. One problem is that the word "floris" is used in these contexts:

  • "FLORIS" as a acronym for the project
  • "floris" as the Python package, this is what is installed from PyPI
  • "floris" as the primary module within the floris.simulation package, i.e. from floris.simulation.floris import Floris. Note that the second "floris" in the import statement was hidden by the __init__ imports at the top level
  • "Floris" as the primary data structure in the floris.simulation package

FlorisInterface as the primary user entry was contextually relevant when FLORIS included interfaces to other simulators like SOWFA, but that functionality was left out in the upgrade to v3. The "Interface" portion remained in the name mostly as a relic but without the context of the other interfaces.

v4 Module-level Redesign

As part of the transition to v4, the directory structure and package architecture of the FLORIS project is redesigned to the following:

classDiagram
    class Floris {
        +FlorisModel
    }
    class core {
        +Core
    }
    class type_dec
    class utilities
    Floris <-- type_dec
    core <-- type_dec
    Floris <-- utilities
    core <-- utilities
    Floris <-- core
Loading

Here, the user-facing component is floris.FlorisModel, and the former tools package is moved up to the top level.

v3 / v4 API Compared

Here's a comparison of the old and new API's for a simple script:

##### v3
import matplotlib.pyplot as plt
import numpy as np

import floris.tools.visualization as wakeviz
from floris.tools import FlorisInterface

fi = FlorisInterface("inputs/gch.yaml")
fi.reinitialize(wind_directions=..., wind_speeds=...)
fi.calculate_wake(yaw_angles=...)
turbine_powers = fi.get_turbine_powers()/1000.

horizontal_plane = fi.calculate_horizontal_plane(height=90.0)
wakeviz.visualize_cut_plane(
    horizontal_plane,
    ax=axarr[0],
    title="Initial setup",
    min_speed=MIN_WS,
    max_speed=MAX_WS
)

wakeviz.show_plots()


##### v4 at this PR

import matplotlib.pyplot as plt
import numpy as np

import floris.visualization as wakeviz
from floris import FlorisModel

fmodel = FlorisModel("inputs/gch.yaml")
fmodel.set(wind_directions=..., wind_speeds=..., yaw_angles=...)
fmodel.run()
turbine_powers = fmodel.get_turbine_powers()/1000.

horizontal_plane = fmodel.calculate_horizontal_plane(height=90.0)
wakeviz.visualize_cut_plane(
    horizontal_plane,
    ax=axarr[0],
    title="Initial setup",
    min_speed=MIN_WS,
    max_speed=MAX_WS
)

wakeviz.show_plots()

Related issues, discussions, PRs

Impacted areas of the software

This touches most files within the floris/ directory. I've tried to keep this pull request scope to variable and class name changes, and I've tried to exclude other minor refactoring.

Things to do

  • First, I'd like feedback on the general design of the API. See examples updated in 940b1b2. Agree to patterns for:
    • Variable name for the FlorisModel instance, currently fmodel
    • Naming conventions for UncertaintyInterface and ParallelComputingInterface, suggested FlorisUncertaintyModel / umodel and FlorisParallelModel / pmodel
    • Is FlorisModel as the main entry point good for everyone?
  • Update API's in remaining packages and examples
    • Find / replace
      • FlorisInterface -> FlorisModel
      • fi = -> fmodel =
      • fi= -> fmodel=
      • fi. -> fmodel.
      • fi, -> fmodel,
      • fi: -> fmodel:
      • fmodel.floris -> fmodel.core
  • Update docs:
    • README
    • Architecture & Design
    • Getting started

@rafmudaf rafmudaf self-assigned this Mar 5, 2024
@rafmudaf rafmudaf added enhancement An improvement of an existing feature v4 Focus of FLORIS v4 examples Changes to FLORIS examples in-progress Work is actively in progress labels Mar 5, 2024
@rafmudaf rafmudaf added this to the v4.0 milestone Mar 5, 2024
@rafmudaf
Copy link
Collaborator Author

rafmudaf commented Mar 5, 2024

@paulf81 @misi9170 @bayc @ejsimley @jfrederik-nrel Please see this pull request, and any conceptual feedback would be helpful. There are a lot of files changed, but the content of the changes is relatively small. Rather than reviewing each file, it would be best to mostly focus on the following locations:

@misi9170
Copy link
Collaborator

misi9170 commented Mar 5, 2024

@rafmudaf I agree with these changes, but I'm not a good data point because we already spoke about this.

One thing though: is there any reason that the floris module containing FlorisModel shouldn't just be called floris_model.py? That would stick to the convention of the file containing a class of the same name, and would also help get rid of this from floris.floris ... business

@rafmudaf
Copy link
Collaborator Author

rafmudaf commented Mar 5, 2024

One thing though: is there any reason that the floris module containing FlorisModel shouldn't just be called floris_model.py? That would stick to the convention of the file containing a class of the same name, and would also help get rid of this from floris.floris ... business

No reason, I just didn't think about it, but I agree and I'll make that change.

@paulf81
Copy link
Collaborator

paulf81 commented Mar 8, 2024

Hi @rafmudaf , taking on the first set of points:

Variable name for the FlorisModel instance, currently fmodel

This works for me, but I might also put forward fm, or even fi as in FlorIs, just keep smaller. But maybe this is a case of just preferring things the way they are and fmodel is much clearer. Also fmodel goes nicely with findex, so maybe I talked myself into fmodel

Naming conventions for UncertaintyInterface and ParallelComputingInterface, suggested FlorisUncertaintyModel / umodel and FlorisParallelModel / pmodel

yes, these are great, and make me like fmodel more

Is FlorisModel as the main entry point good for everyone?

Yes

# Conflicts:
#	examples/01_opening_floris_computing_power.py
#	examples/02_visualizations.py
#	examples/03_making_adjustments.py
#	examples/04_sweep_wind_directions.py
#	examples/32_plot_velocity_deficit_profiles.py
#	floris/floris_model.py
#	floris/flow_visualization.py
#	floris/layout_functions.py
#	floris/layout_visualization.py
#	floris/tools/__init__.py
#	floris/tools/visualization.py
#	floris/uncertainty_interface.py
#	floris/visualization.py
#	tests/floris_model_integration_test.py
@rafmudaf
Copy link
Collaborator Author

Generally, it would be good to keep the scope of this pull request to changing the variable, class, and file names, and then we can do clean up etc in another pull request. This way, when we review the changes here, we can be on the lookout for only a specific set of things.

@paulf81
Copy link
Collaborator

paulf81 commented Mar 11, 2024

Generally, it would be good to keep the scope of this pull request to changing the variable, class, and file names, and then we can do clean up etc in another pull request. This way, when we review the changes here, we can be on the lookout for only a specific set of things.

Sounds good, I think with that I've finished going through and updating all the class references and imports to the new style and all tests and examples are running

@rafmudaf rafmudaf marked this pull request as ready for review March 11, 2024 22:38
# Conflicts:
#	floris/floris_model.py
#	floris/flow_visualization.py
#	tests/floris_model_integration_test.py
@misi9170
Copy link
Collaborator

misi9170 commented Mar 12, 2024

Working through the changes for UncertaintyInterface and ParallelComputingInterface; decided (with @paulf81) to go with:

  • UncertaintyInterface -> UncertainFlorisModel (suggested instantiation ufmodel)
  • ParallelComputingInterface -> ParallelFlorisModel (suggested instantiation pfmodel)

@misi9170
Copy link
Collaborator

Now addresses #834.

Note that support from cut_in_wind_speed and cut_out_wind_speed have been removed from ParallelFlorisModel.get_farm_AEP(). @paulf81 and I have discussed removing support from these from FlorisModel.get_farm_AEP() and UncertainModel.get_farm_AEP() also, but that will come in a separate pull request. These methods are counter to the paradigm that the underlying FlorisModel should contain only the data that is to be computed; instead, we can consider moving such functionality to the WindData objects.

@rafmudaf rafmudaf merged commit e31a4e9 into NREL:v4 Mar 12, 2024
8 checks passed
@rafmudaf rafmudaf deleted the package_name_change branch March 21, 2024 18:46
@misi9170 misi9170 mentioned this pull request Apr 8, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature examples Changes to FLORIS examples in-progress Work is actively in progress v4 Focus of FLORIS v4
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants