FLO-2DSoftware / qgis-flo-2d-plugin

A plugin for pre-processing/post-processing FLO-2D models
7 stars 7 forks source link

Storm Drain Control #1141

Closed FLO-2DKaren closed 5 months ago

FLO-2DKaren commented 10 months ago

Connect the new geopackage table for the control and report groups to the editor so they can be imported, exported and modified.

FLO-2DKaren commented 6 months ago

Robson added the swmm.inp control data table to the geopackage. But we still need to connect it to the system.

The idea behind this issue is to connect that so that we can import data into it and so that it remembers the data that the user adds to the window so it isn't reset each time.

Finally, get with Noemi to see which of those items are default or limited use so we can grey them out.

https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/assets/20424460/2b0844d4-bac1-4458-b6d1-175f1c0cf26f

rpachaly commented 5 months ago

We need to figure out the best way to synchronize data and time (INP) with SIMUL (CON.DAT) and external BC (INP). In previous versions of the plugin, we grayed date and time out in the INP. The plugin calculated end date and time internally as the current date and zero time (START date and time) + simul time. This was exported to the SWMM.INP in the fields for start date and time and end date and time.

Issues with this hardwired synchronization, the BC tables for external flow also use date and time, and they need to be synchronized to the date and time from the INP to correctly impose them during runtime. I spent a lot of time reviewing this for previous plugins, and analyzing the potential different scenarios, and at that time we decided the best option was to use SIMUL from CON.DAT file, use current date and zero time (START date and time in INP), and calculate end date and time using SIMUL, the user needs to adjust BC to match the date and time. We can use a different approach if you want but just be aware of the ramifications of this issue.

Noemi's comment.

rpachaly commented 5 months ago

Hi @FLO-2DNoemi, which of these swmm control variables must be shown to the user?

image

I mean, the FLOW_UNITS can be retrieved from the project and it does not need to be shown in this dialog. The FLOW_ROUTING and the INERTIAL_DAMPING also does not need to be shown here, even if the imported INP file has different values for these options. What are your thoughts on the other options?

My idea is to reduce the amount of information here so the user does not mess up with the SWMM options. Maybe a check box to advanced options to allow more experienced users to have access to more options information.

FLO-2DKaren commented 5 months ago

@FLO-2DNoemi

Hi Noemi,

I asked Robson to connect this to an openable table that can be edited at any time. Not just when the storm drain is exported.

I know that some of these variables are not used because they are hardwired. Can you meet with Robson and discuss this so he can set up this editor.

Robson, I hope to add this editor to a button on the Storm Drain widget next to the Assign nodes button.

rpachaly commented 5 months ago

I'm working on it.

https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/assets/39889306/7c26e379-3760-41ec-80f8-55fb7732af24

FLO-2DNoemi commented 5 months ago

Robson,

Please see my comments below:

FLOW_UNITS needs to be controlled by the CON.DAT file. We do not want the user to be able to enter English units in the surface model and metrics in the SD, or something like that. FLOW_ROUTING needs to be always DYNWAVE ( Dynamic Wave flow routing) START DATE and TIME and END DATE and TIME, this should agree with the SIMUL from CON.DAT file. This was greyed out in previous versions of the plugin because the plugin used the simulation time (SIMUL) and the current date/time to calculate the end date/time. The start date and time and end date and time are irrelevant as long as the difference matches the simulation time. The caveat is that they will also need to agree with date/time for external inflows and SD boundary conditions files. REPORT START DATE/TIME This typically agrees with start date and time. I can not remember if I added the possibility of starting the SD reporting later in the simulation. I will check this and let you know. Report Step, this has to be editable. Inertial Damping: FLO-2D hardwired this to Dampen for all conditions. Force Main equation, this has to be editable.

Check the data input manual for details about the SD control parameters. Email me with questions if you can not find the description of each variable in the manual.

Noemi

On Mon, May 20, 2024 at 9:23 AM Robson Pachaly @.***> wrote:

I'm working on it.

https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/assets/39889306/7c26e379-3760-41ec-80f8-55fb7732af24

— Reply to this email directly, view it on GitHub https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1141#issuecomment-2120454315, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE32O3VCEHX3OPO27G7QJXTZDH2LXAVCNFSM6AAAAABBLVTJOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRQGQ2TIMZRGU . You are receiving this because you were mentioned.Message ID: @.***>

rpachaly commented 5 months ago

@FLO-2DNoemi @FLO-2DKaren I've finished a first version of this. Check this video:

https://flo-2d.sharefile.com/d-s1bbad02e21594e5fa6cfb5adc880f86b

I pushed the branch if you want to test it, but I still need to finish two things:

Let know your thoughts and how we can improve it even more.

FLO-2DNoemi commented 5 months ago

Robson,

I like the idea of hiding the advanced data fields. Some comments for hardwired and default data:

N

On Mon, May 20, 2024 at 4:55 PM Robson Pachaly @.***> wrote:

@FLO-2DNoemi https://github.com/FLO-2DNoemi @FLO-2DKaren https://github.com/FLO-2DKaren I've finished a first version of this. Check this video:

https://flo-2d.sharefile.com/d-s1bbad02e21594e5fa6cfb5adc880f86b

I pushed the branch https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/tree/sd-control if you want to test it, but I still need to finish two things:

  • Quick run
  • Button tooltip

Let know your thoughts and how we can improve it even more.

— Reply to this email directly, view it on GitHub https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1141#issuecomment-2121191736, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE32O3QTVYEFZPSUPYPZPZDZDJPNDAVCNFSM6AAAAABBLVTJOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRRGE4TCNZTGY . You are receiving this because you were mentioned.Message ID: @.***>

FLO-2DKaren commented 5 months ago

@FLO-2DNoemi Do you want Normal_Flow_Limited to be moved to the Hardwired group?

FLO-2DNoemi commented 5 months ago

It is hardwired to BOTH so it should be moved to the hardwire group. Sorry if it was not clear in my previous email.

On Tue, May 21, 2024 at 8:41 AM Karen @.***> wrote:

@FLO-2DNoemi https://github.com/FLO-2DNoemi Do you want Normal_Flow_Limited to be moved to the Hardwired group?

— Reply to this email directly, view it on GitHub https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1141#issuecomment-2122548702, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE32O3WBHFDZ7KWBVVXHKJ3ZDM6IXAVCNFSM6AAAAABBLVTJOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRSGU2DQNZQGI . You are receiving this because you were mentioned.Message ID: @.***>

FLO-2DKaren commented 5 months ago

OK good. I thought that was the case.

I also think you were giving info about Report time being synced to TOUT. That isn't and instruction. Just info right?

FLO-2DNoemi commented 5 months ago

The default value of Report Time should be TOUT, but report values for SD and TOUT for surface results can be different so modifying Report Time should be an option.

On Tue, May 21, 2024 at 8:50 AM Karen @.***> wrote:

OK good. I thought that was the case.

I also think you were giving info about Report time being synced to TOUT. That isn't and instruction. Just info right?

— Reply to this email directly, view it on GitHub https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1141#issuecomment-2122563752, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE32O3WSEJT4T2KKJGFIVLLZDM7H7AVCNFSM6AAAAABBLVTJOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRSGU3DGNZVGI . You are receiving this because you were mentioned.Message ID: @.***>

FLO-2DNoemi commented 5 months ago

Robson,

The data file for UEFCC project includes interior vertices, download the files and take a look:

https://flo-2d.sharefile.com/d-sccf56e06466f4868a6a5f4a37a84adb4

N

On Tue, May 21, 2024 at 8:56 AM Noemi Gonzalez @.***> wrote:

The default value of Report Time should be TOUT, but report values for SD and TOUT for surface results can be different so modifying Report Time should be an option.

On Tue, May 21, 2024 at 8:50 AM Karen @.***> wrote:

OK good. I thought that was the case.

I also think you were giving info about Report time being synced to TOUT. That isn't and instruction. Just info right?

— Reply to this email directly, view it on GitHub https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1141#issuecomment-2122563752, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE32O3WSEJT4T2KKJGFIVLLZDM7H7AVCNFSM6AAAAABBLVTJOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRSGU3DGNZVGI . You are receiving this because you were mentioned.Message ID: @.***>

rpachaly commented 5 months ago

@FLO-2DKaren The SWMM.RPT results are not matching on my testing. I'll probably have this done by Thursday.

FLO-2DKaren commented 5 months ago

I'm going to start testing with an import export test. It shouldn't interfere with your testing.

Blank self help for storm drain testing.zip

rpachaly commented 5 months ago

Karen, I added merged the current master into the branch. The (Disable) column is gone as we expected but I still got the error on the Outfalls.

FLO-2DKaren commented 5 months ago

OK I'll get JJ to fix it. Thanks Robson

FLO-2DKaren commented 5 months ago

tested and approved