Repository: NREL/hescore-hpxml

Issue:
HVAC weighting priority
Right now the priority for weighting factor on HVAC systems is floor area served then fraction of load. If both have fraction and only cooling (for instance) has floor area, it will throw an error because it wants them to be the same instead of choosing fraction for all of them. This should be changed to pick the "lowest common denominator". 

Discussion:

Patch:
diff --git a/docs/source/translation/hvac.rst b/docs/source/translation/hvac.rst
index ec1bc7e9..cdc7169f 100644
--- a/docs/source/translation/hvac.rst
+++ b/docs/source/translation/hvac.rst
@@ -1,26 +1,35 @@
 HVAC Systems
 ############
 
-HEScore allows the definition of up to two HVAC systems which can each include a heating system, cooling system, and duct system.
-To determine which HPXML elements are associated, the ``DistributionSystem`` subelement of
-``HeatingSystem``, ``CoolingSystem``, ``HeatPump`` is used to find the link to ``HVACDistributionSystem``.
-Systems that share the same ``HVACDistributionSystem`` are determined to be the same HVAC system for HEScore.
-
-Sometimes an HVAC system will not share ducts, for instance a central air conditioner and boiler.
-In that case, if each of those systems serve a fraction of the home's load within 5% of each other
-they will be combined into the same HVAC system for HEScore.
-If a ``HeatingSystem`` and ``CoolingSystem`` that are associated with the same ``HVACDistributionSystem`` serve differing
-portions of the house's heating and cooling load, that weight is averaged to find the combined system weight.
-
-If either a heating or cooling system meets all of the load and two systems of the opposite (cooling or heating, respectively)
-are required to meet the same fraction of the load, the larger system is split into two for input into HEScore.
-
-To determine the fraction of the home's heating and cooling load each system serves,
-each HPXML heating or cooling system is required to have ``FloorAreaServed`` or, alternatively ``FracLoadServed``.
-The two combined HVAC systems that serve the greatest portion of the house's load are sent to HEScore.
-
-For details about how each kind of ``HeatingSystem``, ``CoolingSystem``, ``HeatPump``, and ``HVACDistributionSystem``
-are translated into HEScore inputs, see the appropriate subsection:
+HEScore allows the definition of up to two HVAC systems which can each include a
+heating system, cooling system, and duct system. To determine which HPXML
+elements are associated, the ``DistributionSystem`` subelement of
+``HeatingSystem``, ``CoolingSystem``, ``HeatPump`` is used to find the link to
+``HVACDistributionSystem``. Systems that share the same
+``HVACDistributionSystem`` are determined to be the same HVAC system for
+HEScore.
+
+Sometimes an HVAC system will not share ducts, for instance a central air
+conditioner and boiler. In that case, if each of those systems serve a fraction
+of the home's load within 5% of each other they will be combined into the same
+HVAC system for HEScore. If a ``HeatingSystem`` and ``CoolingSystem`` that are
+associated with the same ``HVACDistributionSystem`` serve differing portions of
+the house's heating and cooling load, that weight is averaged to find the
+combined system weight.
+
+If either a heating or cooling system meets all of the load and two systems of
+the opposite (cooling or heating, respectively) are required to meet the same
+fraction of the load, the larger system is split into two for input into
+HEScore.
+
+To determine the fraction of the home's heating and cooling load each system
+serves, each HPXML heating and cooling system is required to have
+``FloorAreaServed`` or, alternatively ``FracLoadServed``. The two combined HVAC
+systems that serve the greatest portion of the house's load are sent to HEScore.
+
+For details about how each kind of ``HeatingSystem``, ``CoolingSystem``,
+``HeatPump``, and ``HVACDistributionSystem`` are translated into HEScore inputs,
+see the appropriate subsection:
 
 .. toctree::
    :maxdepth: 1
diff --git a/hescorehpxml/__init__.py b/hescorehpxml/__init__.py
index 55a58b86..dc737aa9 100644
--- a/hescorehpxml/__init__.py
+++ b/hescorehpxml/__init__.py
@@ -688,7 +688,7 @@ class HPXMLtoHEScoreTranslator(object):
         bldg['zone']['window_construction_same'] = False
         bldg['zone']['zone_wall'] = self._get_building_zone_wall(b, bldg['about'])
         bldg['systems'] = OrderedDict()
-        bldg['systems']['hvac'] = self._get_hvac(b)
+        bldg['systems']['hvac'] = self._get_hvac(b, bldg['about'])
         bldg['systems']['domestic_hot_water'] = self._get_systems_dhw(b)
         generation = self._get_generation(b)
         if generation:
@@ -1678,7 +1678,7 @@ class HPXMLtoHEScoreTranslator(object):
             zone_window['solar_screen'] = max(list(window_sunscreen_areas.items()), key=lambda x: x[1])[0]
         return zone_wall
 
-    def _get_hvac(self, b):
+    def _get_hvac(self, b, bldg_about):
 
         def _get_dict_of_hpxml_elements_by_id(xpathexpr):
             return_dict = {}
@@ -1786,48 +1786,41 @@ class HPXMLtoHEScoreTranslator(object):
             distribution_systems[key] = self._get_hvac_distribution(el)
 
         # Determine the weighting factors
-        def _choose_weighting_factor(systems_dict):
-            weighting_factor_priority = ['_floorarea', '_fracload']
-            found_weighting_factor = False
-            for weighting_factor in weighting_factor_priority:
-                weighting_factor_list = [item[weighting_factor] for item in list(systems_dict.values())]
-                if None not in weighting_factor_list and len(weighting_factor_list) > 0:
-                    found_weighting_factor = True
-                    break
-            if not found_weighting_factor:
-                if len(systems_dict) == 1:
-                    list(systems_dict.values())[0]['_fracload'] = 1.0
-                    weighting_factor_list = [1.0]
-                    weighting_factor = '_fracload'
-                else:
-                    raise TranslationError(
-                        'Every heating/cooling system needs to have either FloorAreaServed or FracLoadServed.')
-            return weighting_factor, sum(weighting_factor_list)
-        if heating_systems:
-            heating_weighting_factor, heating_weight_sum = _choose_weighting_factor(heating_systems)
-        else:
-            heating_weight_sum = 0
-            heating_weighting_factor = None
-        if cooling_systems:
-            cooling_weighting_factor, cooling_weight_sum = _choose_weighting_factor(cooling_systems)
-        else:
-            cooling_weight_sum = 0
-            cooling_weighting_factor = None
 
-        if cooling_systems and heating_systems and heating_weighting_factor != cooling_weighting_factor:
+        # If there's only one heating or cooling system and no weighting elements, fill it in.
+        for heating_or_cooling_systems in (heating_systems, cooling_systems):
+            if len(heating_or_cooling_systems) == 1:
+                for key, hvac_sys in heating_or_cooling_systems.items():
+                    if hvac_sys.get('_floorarea') is None and hvac_sys.get('_fracload') is None:
+                        hvac_sys['_fracload'] = 1.0
+                        hvac_sys['_floorarea'] = bldg_about['conditioned_floor_area']
+
+        # Choose a weighting factor that all the heating and cooling systems use
+        all_systems = []
+        all_systems.extend(heating_systems.values())
+        all_systems.extend(cooling_systems.values())
+        found_weighting_factor = False
+        for weighting_factor in ['_floorarea', '_fracload']:
+            if None not in [x.get(weighting_factor) for x in all_systems]:
+                found_weighting_factor = True
+                break
+        if not found_weighting_factor:
             raise TranslationError(
-                'Every heating/cooling system needs to have either FloorAreaServed or FracLoadServed.')
+                'Every heating/cooling system needs to have either FloorAreaServed or FracHeatLoadServed/FracCoolLoadServed.'  # noqa: E501
+            )
 
-        weight_sum = max(heating_weight_sum, cooling_weight_sum)
-        del heating_weight_sum
-        del cooling_weight_sum
+        # Calculate the sum of the weights (total fraction or floor area)
+        weight_sum = max(
+            sum([x[weighting_factor] for x in heating_systems.values()]),
+            sum([x[weighting_factor] for x in cooling_systems.values()])
+        )
 
         # Ensure that heating and cooling systems attached to the same ducts are within 5% of each other
         # in terms of fraction of the load served.
         for duct_id, (htg_id, clg_id) in list(dist_heating_cooling_map.items()):
             try:
-                htg_weight = old_div(heating_systems[htg_id][heating_weighting_factor], weight_sum)
-                clg_weight = old_div(cooling_systems[clg_id][cooling_weighting_factor], weight_sum)
+                htg_weight = old_div(heating_systems[htg_id][weighting_factor], weight_sum)
+                clg_weight = old_div(cooling_systems[clg_id][weighting_factor], weight_sum)
             except KeyError:
                 continue
             if abs(htg_weight - clg_weight) > 0.051:
@@ -1859,9 +1852,9 @@ class HPXMLtoHEScoreTranslator(object):
         for dist_sys_id, (htg_sys_id, clg_sys_id) in list(dist_heating_cooling_map.items()):
             weights_to_average = []
             if htg_sys_id is not None:
-                weights_to_average.append(old_div(heating_systems[htg_sys_id][heating_weighting_factor], weight_sum))
+                weights_to_average.append(old_div(heating_systems[htg_sys_id][weighting_factor], weight_sum))
             if clg_sys_id is not None:
-                weights_to_average.append(old_div(cooling_systems[clg_sys_id][cooling_weighting_factor], weight_sum))
+                weights_to_average.append(old_div(cooling_systems[clg_sys_id][weighting_factor], weight_sum))
             avg_sys_weight = old_div(sum(weights_to_average), len(weights_to_average))
             hvac_systems_ids.add(IDsAndWeights(htg_sys_id, clg_sys_id, dist_sys_id, avg_sys_weight))
 
@@ -1875,7 +1868,7 @@ class HPXMLtoHEScoreTranslator(object):
                 heatpump_id,
                 heatpump_id,
                 None,
-                old_div(heating_systems[heatpump_id][heating_weighting_factor], weight_sum)))
+                old_div(heating_systems[heatpump_id][weighting_factor], weight_sum)))
 
         # Add the singletons to the list
         for htg_sys_id in singleton_heating_systems:
@@ -1883,13 +1876,13 @@ class HPXMLtoHEScoreTranslator(object):
                 htg_sys_id,
                 None,
                 None,
-                old_div(heating_systems[htg_sys_id][heating_weighting_factor], weight_sum)))
+                old_div(heating_systems[htg_sys_id][weighting_factor], weight_sum)))
         for clg_sys_id in singleton_cooling_systems:
             hvac_systems_ids.add(IDsAndWeights(
                 None,
                 clg_sys_id,
                 None,
-                old_div(cooling_systems[clg_sys_id][cooling_weighting_factor], weight_sum)))
+                old_div(cooling_systems[clg_sys_id][weighting_factor], weight_sum)))
 
         # Split and combine systems by fraction as needed #45
         singleton_heating_systems = []


Test patch:
diff --git a/tests/tests.py b/tests/tests.py
index 1f6fd6cb..a6cd505f 100644
--- a/tests/tests.py
+++ b/tests/tests.py
@@ -410,7 +410,7 @@ class TestOtherHouses(unittest.TestCase, ComparatorBase):
         el = self.xpath('//h:HeatPump[1]/h:FloorAreaServed')
         el.getparent().remove(el)
         self.assertRaisesRegexp(TranslationError,
-                                'Every heating/cooling system needs to have either FloorAreaServed or FracLoadServed',
+                                'Every heating/cooling system needs to have either FloorAreaServed or FracHeatLoadServed/FracCoolLoadServed',  # noqa E501
                                 tr.hpxml_to_hescore_dict)
 
     def test_missing_cooling_weighting_factor(self):
@@ -420,7 +420,7 @@ class TestOtherHouses(unittest.TestCase, ComparatorBase):
         el = self.xpath('//h:CoolingSystem[2]/h:FloorAreaServed')
         el.getparent().remove(el)
         self.assertRaisesRegexp(TranslationError,
-                                'Every heating/cooling system needs to have either FloorAreaServed or FracLoadServed',
+                                'Every heating/cooling system needs to have either FloorAreaServed or FracHeatLoadServed/FracCoolLoadServed',  # noqa E501
                                 tr.hpxml_to_hescore_dict)
 
     def test_bad_duct_location(self):
@@ -1179,7 +1179,7 @@ class TestHVACFractions(unittest.TestCase, ComparatorBase):
         aircond.insert(-1, el)
 
         b = self.xpath('h:Building[1]')
-        hvac_systems = tr._get_hvac(b)
+        hvac_systems = tr._get_hvac(b, {"conditioned_floor_area": 3213})
         hvac_systems.sort(key=lambda x: x['hvac_fraction'])
 
         hvac1 = hvac_systems[0]
@@ -1229,7 +1229,7 @@ class TestHVACFractions(unittest.TestCase, ComparatorBase):
         tr.xpath(ducts2, 'h:SystemIdentifier').attrib['id'] = 'ducts2'
 
         b = self.xpath('h:Building[1]')
-        hvac_systems = tr._get_hvac(b)
+        hvac_systems = tr._get_hvac(b, {"conditioned_floor_area": 2600})
         hvac_systems.sort(key=lambda x: x['hvac_fraction'])
 
         hvac1 = hvac_systems[0]
@@ -1275,7 +1275,7 @@ class TestHVACFractions(unittest.TestCase, ComparatorBase):
         clgsys_floor_area_el.text = str(total_floor_area)
 
         b = self.xpath('h:Building[1]')
-        hvac_systems = tr._get_hvac(b)
+        hvac_systems = tr._get_hvac(b, {"conditioned_floor_area": 2600})
         hvac_systems.sort(key=lambda x: x['hvac_fraction'])
 
         hvac1 = hvac_systems[0]
@@ -1325,7 +1325,7 @@ class TestHVACFractions(unittest.TestCase, ComparatorBase):
 
         # Get HEScore inputs and sort by fraction
         b = self.xpath('h:Building[1]')
-        hvac_systems = tr._get_hvac(b)
+        hvac_systems = tr._get_hvac(b, {"conditioned_floor_area": 2600})
         hvac_systems.sort(key=lambda x: x['hvac_fraction'])
 
         hvac1 = hvac_systems[0]
@@ -1348,7 +1348,21 @@ class TestHVACFractions(unittest.TestCase, ComparatorBase):
         frac_cool_load_served.text = '1.0'
         clg_sys_eff.addprevious(frac_cool_load_served)
         b = self.xpath('h:Building[1]')
-        tr._get_hvac(b)
+        tr._get_hvac(b, {"conditioned_floor_area": 2400})
+
+    def test_different_weighting_factors(self):
+        tr = self._load_xmlfile('hescore_min')
+        htg_sys = self.xpath('//h:HeatingSystem')
+        frac_heat_load_served = etree.SubElement(htg_sys, tr.addns('h:FractionHeatLoadServed'))
+        frac_heat_load_served.text = '1.0'
+        heating_floor_area_served = etree.SubElement(htg_sys, tr.addns('h:FloorAreaServed'))
+        heating_floor_area_served.text = '2400'
+        clg_sys_eff = self.xpath('//h:CoolingSystem/h:AnnualCoolingEfficiency')
+        frac_cool_load_served = etree.Element(tr.addns('h:FractionCoolLoadServed'))
+        frac_cool_load_served.text = '1.0'
+        clg_sys_eff.addprevious(frac_cool_load_served)
+        b = self.xpath('h:Building[1]')
+        tr._get_hvac(b, {"conditioned_floor_area": 2400})
 
 
 class TestPhotovoltaics(unittest.TestCase, ComparatorBase):