Speed up turbine loading operation (#966)

* Instantiate Turbine objects only once for each different type of turbine.

* Raise clear error of turbine_type is same but dictionaries do not match

* Clean up comments.

* Add test to check that turbine_definitions and turbine_map reflect changes as expected.

* Use private attribute _turbine_definition_cache throughout.

* use factory=dict on attrs attribute to avoid need to assign empty {}

* Add factory=list to _turbine_types private attribute to ensure acts as expected.

floris/core/farm.py

@@ -120,6 +120,10 @@ class Farm(BaseClass):
 
     internal_turbine_library: Path = field(init=False, default=default_turbine_library_path)
 
+    # Private attributes
+    _turbine_types: List = field(init=False, validator=iter_validator(list, str), factory=list)
+    _turbine_definition_cache: dict = field(init=False, factory=dict)
+
     def __attrs_post_init__(self) -> None:
         # Turbine definitions can be supplied in three ways:
         # - A string selecting a turbine in the floris turbine library
@@ -134,21 +138,28 @@ def __attrs_post_init__(self) -> None:
         # This allows to read the yaml input files once rather than every time they're given.
         # In other words, if the turbine type is already in the cache, skip that iteration of
         # the for-loop.
-        turbine_definition_cache = {}
+
         for t in self.turbine_type:
             # If a turbine type is a dict, then it was either preprocessed by the yaml
             # library to resolve the "!include" or it was set in a script as a dict. In either case,
             # add an entry to the cache
             if isinstance(t, dict):
-                if t["turbine_type"] in turbine_definition_cache:
-                    continue
-                turbine_definition_cache[t["turbine_type"]] = t
+                if t["turbine_type"] in self._turbine_definition_cache:
+                    if self._turbine_definition_cache[t["turbine_type"]] == t:
+                        continue # Skip t if already loaded
+                    else:
+                        raise ValueError(
+                            "Two different turbine definitions have the same name: "\
+                            f"'{t['turbine_type']}'. "\
+                            "Please specify a unique 'turbine_type' for each turbine definition."
+                        )
+                self._turbine_definition_cache[t["turbine_type"]] = t
 
             # If a turbine type is a string, then it is expected in the internal or external
             # turbine library
             if isinstance(t, str):
-                if t in turbine_definition_cache:
-                    continue
+                if t in self._turbine_definition_cache:
+                    continue # Skip t if already loaded
 
                 # Check if the file exists in the internal and/or external library
                 internal_fn = (self.internal_turbine_library / t).with_suffix(".yaml")
@@ -174,7 +185,7 @@ def __attrs_post_init__(self) -> None:
                         f"The turbine type: {t} does not exist in either the internal or"
                         " external turbine library."
                     )
-                turbine_definition_cache[t] = load_yaml(full_path)
+                self._turbine_definition_cache[t] = load_yaml(full_path)
 
         # Convert any dict entries in the turbine_type list to the type string. Since the
         # definition is saved above, we can make the whole list consistent now to use it
@@ -184,23 +195,23 @@ def __attrs_post_init__(self) -> None:
         # types must be used. If we modify that directly and change its shape, recreating this
         # class with a different layout but not a new self.turbine_type could cause the data
         # to be out of sync.
-        _turbine_types = [
+        self._turbine_types = [
             copy.deepcopy(t["turbine_type"]) if isinstance(t, dict) else t
             for t in self.turbine_type
         ]
 
         # If 1 turbine definition is given, expand to N turbines; this covers a 1-turbine
         # farm and 1 definition for multiple turbines
-        if len(_turbine_types) == 1:
-            _turbine_types *= self.n_turbines
+        if len(self._turbine_types) == 1:
+            self._turbine_types *= self.n_turbines
 
         # Check that turbine definitions contain any v3 keys
-        for t in _turbine_types:
-            check_turbine_definition_for_v3_keys(turbine_definition_cache[t])
+        for _, v in self._turbine_definition_cache.items():
+            check_turbine_definition_for_v3_keys(v)
 
         # Map each turbine definition to its index in this list
         self.turbine_definitions = [
-            copy.deepcopy(turbine_definition_cache[t]) for t in _turbine_types
+            copy.deepcopy(self._turbine_definition_cache[t]) for t in self._turbine_types
         ]
 
     @layout_x.validator
@@ -285,7 +296,10 @@ def construct_turbine_correct_cp_ct_for_tilt(self):
         )
 
     def construct_turbine_map(self):
-        self.turbine_map = [Turbine.from_dict(turb) for turb in self.turbine_definitions]
+        turbine_map_unique = {
+            k: Turbine.from_dict(v) for k, v in self._turbine_definition_cache.items()
+        }
+        self.turbine_map = [turbine_map_unique[k] for k in self._turbine_types]
 
     def construct_turbine_thrust_coefficient_functions(self):
         self.turbine_thrust_coefficient_functions = {

tests/farm_unit_test.py

@@ -115,6 +115,44 @@ def test_check_turbine_type(sample_inputs_fixture: SampleInputs):
     assert len(farm.turbine_type) == 5
     assert len(farm.turbine_definitions) == 5
 
+    # Check that error is correctly raised if two turbines have the same name
+    farm_data = deepcopy(sample_inputs_fixture.farm)
+    external_library = Path(__file__).parent / "data"
+    farm_data["layout_x"] = np.arange(0, 500, 100)
+    farm_data["layout_y"] = np.zeros(5)
+    turbine_def = load_yaml(external_library / "nrel_5MW_custom.yaml")
+    turbine_def_mod = deepcopy(turbine_def)
+    turbine_def_mod["hub_height"] = 100.0 # Change the hub height of the last turbine
+    farm_data["turbine_type"] = [turbine_def]*4 + [turbine_def_mod]
+    with pytest.raises(ValueError):
+        farm = Farm.from_dict(farm_data)
+    # Check this also raises an error in a nested level of the turbine definition
+    turbine_def_mod = deepcopy(turbine_def)
+    turbine_def_mod["power_thrust_table"]["wind_speed"][-1] = -100.0
+    farm_data["turbine_type"] = [turbine_def]*4 + [turbine_def_mod]
+    with pytest.raises(ValueError):
+        farm = Farm.from_dict(farm_data)
+
+    # Check that no error is raised, and the expected hub heights are seen,
+    # if turbine_type is correctly updated
+    farm_data = deepcopy(sample_inputs_fixture.farm)
+    external_library = Path(__file__).parent / "data"
+    farm_data["layout_x"] = np.arange(0, 500, 100)
+    farm_data["layout_y"] = np.zeros(5)
+    turbine_def = load_yaml(external_library / "nrel_5MW_custom.yaml")
+    turbine_def_mod = deepcopy(turbine_def)
+    turbine_def_mod["hub_height"] = 100.0 # Change the hub height of the last turbine
+    turbine_def_mod["turbine_type"] = "nrel_5MW_custom_2"
+    farm_data["turbine_type"] = [turbine_def]*4 + [turbine_def_mod]
+    farm = Farm.from_dict(farm_data)
+    for i in range(4):
+        assert farm.turbine_definitions[i]["hub_height"] == turbine_def["hub_height"]
+    assert farm.turbine_definitions[-1]["hub_height"] == 100.0
+    farm.construct_turbine_map()
+    for i in range(4):
+        assert farm.turbine_map[i].hub_height == turbine_def["hub_height"]
+    assert farm.turbine_map[-1].hub_height == 100.0
+
     # Duplicate type found in external and internal library
     farm_data = deepcopy(sample_inputs_fixture.farm)
     external_library = Path(__file__).parent / "data"