Flow and Routing Stats Bug

[jennwuu]

I am seeing an issue with continuity error in the API. At the moment, I think we are reporting incorrect continuity error. While going through the code, I see that we get the percent error and multiply it by 100. This is incorrect because the data structure is storing percentage already. https://github.com/OpenWaterAnalytics/Stormwater-Management-Model/blob/93891ff1e3775073895a3b96f38fa6999e961ed9/src/solver/objects.h#L852

This is not the main concern. The main issue is that continuity error is only calculated when the .rpt file is being written by function swmm_getSystemRunoffTotals (and other functions) https://github.com/OpenWaterAnalytics/Stormwater-Management-Model/blob/93891ff1e3775073895a3b96f38fa6999e961ed9/src/solver/massbal.c#L284.

The rpt reporting process only starts when swmm_ends is called. However, after rpt is written, the memory is freed. https://github.com/OpenWaterAnalytics/Stormwater-Management-Model/blob/93891ff1e3775073895a3b96f38fa6999e961ed9/src/solver/swmm5.c#L519-L525 Therefore, any continuity values (and potentially other variables like finalStorage and finalSnowCover and snowRemoved) that we report through the toolkitapi are incorrect.

We saw this issue reported last year here: https://github.com/OpenWaterAnalytics/pyswmm/issues/234

Potential solution to address the issue identified above: https://github.com/jennwuu/Stormwater-Management-Model/commit/a0aa668692bba4e3d4280e764c7279e1c756dc0f

[michaeltryby]

So the problem is that stats are being computed and written to the report because that's how the program was originally conceived. Now that there is an API and we want more flexible access to stats we need to decouple stats computation from report writing.

[jennwuu]

I am looking for review regarding this issue. Looking to see if anyone can review the proposed solution here: https://github.com/jennwuu/Stormwater-Management-Model/commit/a0aa668692bba4e3d4280e764c7279e1c756dc0f

[jennwuu]

@bemcdonnell @michaeltryby @abhiramm7 @cbuahin I want to follow up this issue and see if anyone can review the proposed solution above.

[bemcdonnell]

@jennwuu, I can review it today. I think the approach is to create a simple function to compute stats and both the api can call it (while the model is running) and the rpt writer can call it. But your PR could be more. You could look at see all the values being calculated in the rpt writer and make them all functions.

[abhiramm7]

@jennwuu did we address this issue? Looks like you had a fix ready to go.

[jennwuu]

@abhiramm7 No, I proposed a solution, but it needs review.

[jennwuu]

https://github.com/jennwuu/Stormwater-Management-Model/commit/a0aa668692bba4e3d4280e764c7279e1c756dc0f

[abhiramm7]

cool thanks @jennwuu :)

[karosc]

merged @jennwuu 's commit in @354. Thanks for the great work!