Merge pull request #8931 from gem/missingtag

Give a better error message in case aggregate_by is not in the tags

debian/changelog

@@ -1,6 +1,8 @@
   [Paolo Tormene]
   * Raise an error if files specified in site_model_file do not have the same
     headers
+  * Give a better error message for risk calculations where the aggregate_by
+    tag is specified but it doesn't exist in the exposure model
 
   [Michele Simionato]
   * Improved error message when quantiles are not supported in event based risk

openquake/calculators/base.py

@@ -572,7 +572,8 @@ def pre_execute(self):
             haz_sitecol = readinput.get_site_collection(oq, self.datastore)
             self.load_crmodel()  # must be after get_site_collection
             self.read_exposure(haz_sitecol)  # define .assets_by_site
-            self.datastore.create_df('_poes', readinput.Global.pmap.to_dframe())
+            self.datastore.create_df('_poes',
+                                     readinput.Global.pmap.to_dframe())
             self.datastore['assetcol'] = self.assetcol
             self.datastore['full_lt'] = fake = logictree.FullLogicTree.fake()
             self.datastore['trt_rlzs'] = U32([[0]])
@@ -678,7 +679,9 @@ def read_exposure(self, haz_sitecol):  # after load_risk_model
         .sitecol, .assetcol
         """
         oq = self.oqparam
-        self.sitecol, self.assetcol, discarded = readinput.get_sitecol_assetcol(
+        (self.sitecol,
+         self.assetcol,
+         discarded) = readinput.get_sitecol_assetcol(
             oq, haz_sitecol, self.crmodel.loss_types, self.datastore)
         # this is overriding the sitecol in test_case_miriam
         self.datastore['sitecol'] = self.sitecol
@@ -845,7 +848,6 @@ def _read_risk_data(self):
             if self.sitecol and oq.imtls:
                 logging.info('Read N=%d hazard sites and L=%d hazard levels',
                              len(self.sitecol), oq.imtls.size)
-
         if oq_hazard:
             parent = self.datastore.parent
             if 'assetcol' in parent:
@@ -992,7 +994,7 @@ def store_source_info(self, source_data):
             recs, source_reader.source_info_dt)
 
     def post_process(self):
-       	"""
+        """
         Run postprocessing function, if any
         """
         oq = self.oqparam
@@ -1111,7 +1113,8 @@ def import_gmfs_csv(dstore, oqparam, sitecol):
     if names[0] == 'rlzi':  # backward compatibility
         names = names[1:]  # discard the field rlzi
     names = [n for n in names if n != 'custom_site_id']
-    imts = [name.lstrip('gmv_') for name in names if name not in ('sid', 'eid')]
+    imts = [name.lstrip('gmv_')
+            for name in names if name not in ('sid', 'eid')]
     oqparam.hazard_imtls = {imt: [0] for imt in imts}
     missing = set(oqparam.imtls) - set(imts)
     if missing:

openquake/calculators/tests/event_based_risk_test.py

@@ -22,6 +22,7 @@
 
 from openquake.baselib.general import gettemp
 from openquake.baselib.hdf5 import read_csv
+from openquake.hazardlib import InvalidFile
 from openquake.hazardlib.source.rupture import get_ruptures
 from openquake.commonlib import logs, readinput
 from openquake.calculators.views import view, text_table
@@ -83,6 +84,13 @@ def test_case_1(self):
             self.assertEqualFiles('expected/%s' % strip_calc_id(fname), fname,
                                   delta=1E-5)
 
+    def test_case_1_missing_occupancy(self):
+        with self.assertRaises(InvalidFile) as ctx:
+            self.run_calc(case_1.__file__, 'job_missing_occupancy.ini')
+        self.assertIn('Missing tag "occupancy" in', str(ctx.exception))
+        self.assertIn('qa_tests_data/event_based_risk/case_1/exposure.csv',
+                      str(ctx.exception))
+
     def test_case_1_ins(self):
         # no aggregation
         self.run_calc(case_1.__file__, 'job2.ini')

openquake/calculators/tests/event_risk_test.py

@@ -107,7 +107,7 @@ def test_case_5(self):
         self.run_calc(case_5.__file__, 'job.ini')
         rbe = self.calc.datastore.read_df('risk_by_event')
         self.assertEqual(len(rbe), 0)
-        
+
     def test_case_master(self):
         self.run_calc(case_master.__file__, 'job.ini')
         calc0 = self.calc.datastore  # single file event_based_risk

openquake/commonlib/readinput.py

@@ -955,7 +955,8 @@ def get_exposure(oqparam, h5=None):
             oqparam.ignore_missing_costs,
             by_country='country' in asset.tagset(oqparam.aggregate_by),
             errors='ignore' if oqparam.ignore_encoding_errors else None,
-            infr_conn_analysis=oqparam.infrastructure_connectivity_analysis)
+            infr_conn_analysis=oqparam.infrastructure_connectivity_analysis,
+            aggregate_by=oqparam.aggregate_by)
     return exposure
 
 

openquake/qa_tests_data/event_based_risk/case_1/job_missing_occupancy.ini

@@ -0,0 +1,56 @@
+[general]
+random_seed = 23
+master_seed = 42
+description = Event Based Risk QA Test 1
+calculation_mode = event_based_risk
+structural_vulnerability_file = vulnerability_model_stco.xml
+nonstructural_vulnerability_file = vulnerability_model_nonstco.xml
+exposure_file = exposure1.zip
+discard_assets = true
+aggregate_by = occupancy
+
+region = 81.1 26, 88 26, 88 30, 81.1 30
+
+asset_hazard_distance = 20
+
+conditional_loss_poes = 0.1
+
+quantiles = 0.25
+
+avg_losses = true
+
+sites = 81.2985 29.1098, 83.082298 27.9006, 85.747703 27.9015
+
+[logic_tree]
+
+number_of_logic_tree_samples = 0
+
+[erf]
+
+# km
+rupture_mesh_spacing = 5
+width_of_mfd_bin = 0.3
+# km
+area_source_discretization = 10
+
+[site_params]
+
+reference_vs30_type = measured
+reference_vs30_value = 760.0
+reference_depth_to_2pt5km_per_sec = 5.0
+reference_depth_to_1pt0km_per_sec = 100.0
+
+[calculation]
+source_model_logic_tree_file = source_model_logic_tree.xml
+gsim_logic_tree_file = gmpe_logic_tree.xml
+# years
+investigation_time = 50.0
+ses_per_logic_tree_path = 20
+truncation_level = 3
+# km
+maximum_distance = 100.0
+return_periods = [30, 60, 120, 240, 480, 960]
+individual_rlzs = true
+
+[output]
+export_dir = /tmp

openquake/risklib/asset.py

@@ -783,7 +783,7 @@ def check_exposure_for_infr_conn_analysis(df, fname):
 def read_exp_df(fname, calculation_mode='', ignore_missing_costs=(),
                 check_dupl=True, by_country=False, asset_prefix='',
                 tagcol=None, errors=None, infr_conn_analysis=False,
-                monitor=None):
+                aggregate_by=None, monitor=None):
     logging.info('Reading %s', fname)
     exposure, assetnodes = _get_exposure(fname)
     if tagcol:
@@ -817,7 +817,15 @@ def read_exp_df(fname, calculation_mode='', ignore_missing_costs=(),
                                'value-number': df['value-number']})
         if infr_conn_analysis:
             check_exposure_for_infr_conn_analysis(df, fname)
-
+        if aggregate_by:
+            for taglist in aggregate_by:
+                for tag in taglist:
+                    if tag == 'site_id':
+                        # 'site_id' is added later in _get_mesh_assets
+                        continue
+                    if (tag not in df.columns
+                            and f'value-{tag}' not in df.columns):
+                        raise InvalidFile(f'Missing tag "{tag}" in {fname}')
         df['id'] = asset_prefix + df.id
         dfs.append(df)
 
@@ -893,7 +901,7 @@ def check(fname):
     @staticmethod
     def read_all(fnames, calculation_mode='', ignore_missing_costs=(),
                  check_dupl=True, tagcol=None, by_country=False, errors=None,
-                 infr_conn_analysis=False):
+                 infr_conn_analysis=False, aggregate_by=None):
         """
         :returns: an :class:`Exposure` instance keeping all the assets in
             memory
@@ -917,7 +925,7 @@ def read_all(fnames, calculation_mode='', ignore_missing_costs=(),
                 prefix = ''
             allargs.append((fname, calculation_mode, ignore_missing_costs,
                             check_dupl, by_country, prefix, tagcol, errors,
-                            infr_conn_analysis))
+                            infr_conn_analysis, aggregate_by))
         exp = None
         dfs = []
         for exposure, df in itertools.starmap(read_exp_df, allargs):