NRCan / CanFlood

Flood Risk modelling toolbox for Canada
MIT License
29 stars 7 forks source link

AssertionError: rtail value for 1 in 2 year flood #69

Closed tsunamiresearch closed 2 years ago

tsunamiresearch commented 2 years ago

When running an L1 assessment for multiple flood scenarios (1:2yr, 1:5yr, 1:10yr, 1:20yr, 1:100yr), I receive an AssertionError: passed rtail value (0.50) not > max aep (0.50) error. (I suspect it would also fail for L2, but haven't tested that.)

Exact error as follows:

AssertionError: passed rtail value (0.50) not > max aep (0.50)
Traceback (most recent call last):
  File "C:\Users\XXX\AppData\Roaming\QGIS\QGIS3\profiles\default\python\plugins\canflood\model\dialog.py", line 164, in run_risk1
    res_ttl, res_df = model.run(res_per_asset=self.checkBox_r1_rpa.isChecked())
  File "C:\Users\XXX\AppData\Roaming\QGIS\QGIS3\profiles\default\python\plugins\canflood\model\risk1.py", line 268, in run
    res_df = self.calc_ead(bres_df)
  File "C:\Users\XXX\AppData\Roaming\QGIS\QGIS3\profiles\default\python\plugins\canflood\model\riskcom.py", line 807, in calc_ead
    assert aep_val > df.columns.max(), 'passed rtail value (%.2f) not > max aep (%.2f)'%(
AssertionError: passed rtail value (0.50) not > max aep (0.50)

Removing the 1:2yr scenario and updating related elements clears the error.

Workaround: Before running the L1 assessment, change the value for the following line in the control file from 0.5 to 0.6: rtail = 0.5 #EAD extrapolation: right trail treatment (high prob low damage)

While this workaround helps, we will be conducting several coastal flood studies with modelling for the 1:2yr event for several locations across Canada and it would be nice to get this working by default for high probability, low impact events.

cefect commented 2 years ago

Hi Ryan, sorry to hear you're having issues with the rtail parameter. This tells CanFlood how to complete the right tail of the risk curve. By specifying a value, you're telling CanFlood "at this probability there is zero damage"... so it throws an error when you also include an event of equal or greater probability in the event table. Therefore, you should specify a higher value...as you've done. i.e., this is not a workaround but working as intended. We'll try and clarify the documentation.

the parameter is described here.

Let me know if this clarifies things and I'll close the issue.

tsunamiresearch commented 2 years ago

Hi Seth, thanks for the quick feedback!

I agree that an update to the documentation to clarify would be very helpful. That said, there is nothing the user experiences at any point leading up to the error that would lead them to expect such an error. There is also nothing in the error message that would provide them hints on where to go to address the issue themselves. So, might I suggest the following changes as well:

Beyond the above, while 0.5 seems quite a reasonable limit for most situations, it is arbitrary and can therefore be changed. Depending on how things are set up on the backend, using a greater-than-or-equal check instead or equals (i.e, rtail value (0.50) ≥ max aep (0.50)) or changing to a slightly higher default value will also guarantee this issue doesn't occur at all without any significant impact on other useres. This would help user experience to better align with user expectations. Just my 2 cents.

cefect commented 2 years ago

Hi Ryan, thank you for the feedback. This is very useful. I added a suggestion to the error message and a tooltip to the build dialog - event variables tab (see PR 70). A nicer fix would probably be to add the parameter (rtail and ltail) directly to the UI (but this would take more effort). Adding a check to the validation tool (which is a good idea as this is what it's there for) would also take more effort as this tool is under-developed.

As for the parameter itself, it needs to be greater than the largest AEP (most probable) event specified in 'evals' (> not >=). 0.5 (2-year) is the value in the control file template, which acts as a default when building a new model from the UI (you can customize this by editing this file). Along with changing this value, there are four other key word options (see control file summary) the user could specify in the control file.

Apart from this, I'm a bit suspicious of using such a small/probable event in your model. Do you really have impacts in half of all previous years?

Cheers,

tsunamiresearch commented 2 years ago

Thanks Seth, those sound like reasonable future changes. I'll definitely modify the pars file.

Unfortuntely we are seeing impacts from these "regular" events in our work in flat-laying areas in the Arctic today, resulting in damage to homes or the need to relocate homes as coastal erosion increases. Of greater concerns is that a 1:2yr flood starts to cover the same areas that a 1:5, 1:10, or even 1:20 year flood does today as we incorporate sea level rise into our future-looking scenarios in these areas.

Thanks again!