USEPA / Stormwater-Management-Model

Dynamic hydrology-hydraulic water quality simulation model
236 stars 174 forks source link

Missing system rainfall when simulating RDII #20

Closed Tiehong closed 1 year ago

Tiehong commented 5 years ago

In gage_validate() method, new function was added to ignore unused gage. This will exclude gages for RDII which use flag is set in a later step. As a result, the gage.coGage is not set correctly which will advance time series table pointer accidentally.

    // --- no validation for an unused gage
    if ( !Gage[j].isUsed ) return;

Here is a sample project for the issue:

Rainfall.zip

michaeltryby commented 5 years ago

@Tiehong thank you very much for identifying this issue. You are invited to issue a PR with the fix for it if you are so inclined.

dickinsonre commented 5 years ago

@Tiehong and @michaeltryby I concur with this finding. It has been an issue at Innovyze for InfoSWMM since we updated to EPA SWMM 5.1013 and we made the same change as suggested for gage_validate to fix the issue. We saw dramatic differences between SWMM 5.1.012 and SWMM 5.103 for RDII in most models. and now get the same answer for RDII with co-gages. My apologies, I thought I had added this issue to the Github.

This also points the tremendous value a complete testing program adds to EPA SWMM. Just for completeness, let me point out that if you had testing models with one gage and one RDII UH there is no difference between SWMM 5.1.012 and SWMM 5.1.03.

The reported issue only happens when two or more RDII UH's shares the same raingage as mentioned in this blog. My apologies again for not mentioning it here

https://swmm5.org/2019/03/29/new-features-in-innovyze-swmm-14-7-update-1/

martensgd commented 5 years ago

I'm not sure this is the same issue (likely ?)

Something has changed between SWMM 5.1.012 and 5.1.013 that causes the RUNOFF from a SUBCATCHMENT (or RDII)to be reduced when RDII is active and the following is the case:

(1) The SUBCATCHMENTs and the HYDROGRAPHs each have their own distinct RAINGAGE (2) The RAINGAGEs all refer to the same TIMESERIES (either in the *.inp or as an external file) (3) Similarly, if there are more than one HYDROGRAPH then the RDII timeseries are also erroneous (if there is only one it looks fine) regardless of the presence of SUBCATCHMENTS

Note: This error does NOT occur when: (i) using an external SWMM Rainfall FILE (ii) SUBCATCHMENTS and HYDROGRAPHS use the same RAINGAGE

A sample input file that produces this issue is attached.

test_RUNOFF_RDII.txt

dickinsonre commented 5 years ago

Thanks for the file @martensgd Next step is to actually try this using v12 and v13.

dickinsonre commented 5 years ago

This is what I get with v12 for RDII in your test model

EPA STORM WATER MANAGEMENT MODEL - VERSION 5.1 (Build 5.1.012)- QA/QC

Test SWMM 5.1.013 RDII/RUNOFF related error with multiple RAINGAGES refering to the same timeseries

WARNING 04: minimum elevation drop used for Conduit 2 WARNING 04: minimum elevation drop used for Conduit 3

** Volume Volume Rainfall Dependent I/I acre-feet 10^6 gal ** --------- --------- Sewershed Rainfall ...... 44.662 14.554 RDII Produced ........... 1.579 0.515 RDII Ratio .............. 0.035

Which is the same as I get in v13. However, it looks like the runoff has been reduced from 0.666 inches in v12 to 0.16 in v13 due to the smaller rainfall in v13 of 0.65 inches vs 2.68 inches in v12

martensgd commented 5 years ago

Yes I also get these values for v12, however for v13 I get lower rainfall and RDII:

** Volume Volume Rainfall Dependent I/I acre-feet 10^6 gal ** --------- --------- Sewershed Rainfall ...... 22.449 7.315 RDII Produced ........... 0.794 0.259 RDII Ratio .............. 0.035

I think it is of interest that this problem only occurs when using rainfall as a TIMESERIES (defined in the .inp or as an external file) and it does NOT occur when using an external rainfall formatted text FILE or a rainfall interface FILE (.rff).

The proposed solution can be tested using this case.

dickinsonre commented 5 years ago

@martensgd actually the proposed solution does not work, I get the same answers in native v13 and modified v13. The issue is the rainfall, somehow, the RDII is causing the rainfall not to be the same. I think one of the rainfall files or the wrong rainfall files is not being used. This can be checked by adding a fprintf debug statement to the actual time series used by each subcatchment,

martensgd commented 5 years ago

It appears to lose rainfall for every SUBCATCHMENT exactly the same - as can seen in the report file's Subcatchment Runoff Summary. (This is seen in my models with several hundred SUBCATCHMENTs also.)

dickinsonre commented 5 years ago

I am still looking at this but if you turn off RDII in v13 you get the same runoff in v12 and v13. If you duplicate your time series and use a differently named time series in v13 you get the same runoff in v12 and v13 even with RDII. RDII is affecting the temp rainfall binary file.

michaeltryby commented 1 year ago

Closed in Release v5.1.14